Re: cvs: php-src /ext/curl interface.c

Hello Pierre, Tuesday, November 11, 2008, 12:01:38 PM, you wrote: > hi, > On Tue, Nov 11, 2008 at 11:51 AM, Marcus Boerger <helly@php.net> wrote: >> This is extremely bad code. We have tons of better APIs to do what you >> want. And besides wouldn't it be better to list all those entries with >> enabled/disabled as table rows anyway? > I do not think adding a 30 lines table is better. About being an > extremely bad code, in which way? What will you use then? Each of > these features or protocols are present or not. The maximum size is > known and the buffer is large enough to contain them. >> (spprintf, snprintf, strcat, smart-strings,...) >> (the problem is that adding something will mostlikely result in crashes as >> ppl will add the necessary three lines and be happy because something else >> is disabled for them. Then someone having all enabled gets crashes because >> they forgot to increase the size of the string. > The problem is you do not know before hand which features are enabled. > As I agree that dynamic allocation for the protocols string would be > better, there is no need to use it now or in the mid term. The size of > the tmp buffer is large enough to support much more protocols than > curl or features can ever have. > One thing I forgot to add is to check if n > buffer size, it will > solve the problem you are refering to. In case n > buffersize you already crash. And as said you are creating a maintenance issue here. And besides your own one, people will copy this code and fail calculating the buffer correct. For one, ppl often do not know that there is a limited buffer or that a limited buffer could fail. And even if you know how to do it correct. Someone else taking on won't. p.s.: No one will ever need more that 640K RAM :-) Best regards, Marcus