Daniel hi,
it wasn't my intention to make your life hard. I too enjoy working
with(and for) libcurl and I want to write something useful for it.
You'll hear soon from me!

2013/2/22 Daniel Stenberg <daniel_at_haxx.se>:
> On Thu, 21 Feb 2013, Chrysovaladis Datsios wrote:
>
>> I read no prohibition in putting read function inside main. It's just a
>> personal decision. I believe if the patch gets accepted a more "formal" code
>> example will be written.
>
>
> It isn't C89 compliant code so it couldn't be used by us as-is, and we can't
> accept the code without some test code being provided along with the patch.
>
> You're not making this very easy for me. I spend time to help you cleanup
> things, I make a patch that is a step forward and then hand the work back to
> you. You then (seemingly) completely ignore my work and start over with your
> stuff and present me an updated patch that isn't based on what I did for
> you.
>
> So now I need to redo that work again and show it back to you _again_ only
> to see you throw it away? While I enjoy working on curl, repeatedly doing
> the same things is not my favorite hobby. If you don't want to base your
> work on my improvements, then you need to at least look at my patch and
> incorporate back some of my changes. I've fixed your "funny" indenting and I
> updated copyright years and some little tidbits. And I made a full and
> proper commit message that seemed at least something to base further edits
> on.
>
> Please make a full git commit with your changes when you send a patch,
> including a full commit message and proper author.
>
> Now, over to my review of the contents of the 21feb incarnation of this
> patch (my comments are in a somewhat random order):
>
> 1 - it doesn't follow our indent style and you use 'char * name' style of
> declarations that we don't use.
>
> 2 - why calloc() a 100K buffer instead of just malloc? And you don't have
> to
> typecast the return value of calloc() (or malloc) to char *.
>
> 3 - I would appreciate a way that starts off with a much smaller buffer but
> that can grow if needed. 100K seems excessive in just about all
> sensible use-cases.
>
> 4 - Why do you need 'trailer_headers' to be in the connectdata struct? I
> could easily remove it and just have it as a local variable and nothing
> seems to break? That leads me into...
>
> 5 - Please consider a few test cases that proves that your code is working.
>
> 6 - You don't check the return code from calloc() or realloc() which thus
> will crash the lib on out of memory situations. We can't have that and
> this would be easily tested if we had a test case.
>
> 7 - the realloc(NULL) case below the header look is a mystery to me. Why
> realloc? Also, is that not a memory leak? Would've been shown if we had
> test cases...
>
> 8 - Also, the realloc() does two strlen() calls, the first being
> strlen(trailer_headers_data). Isn't that superfluos as that's exactly
> the
> length 'trlen' holds at that point?
>
> 9 - then you call strlen() again on the line below. Remember that you allow
> 100K of data in there, doing strlen() is not a quick operation then...
> In general you do a lot of repeated strlen() calls in that logic.
>
> 10 - the linked list a callback returns to libcurl, where is that freed?
>
> --
>
> / daniel.haxx.se
>
> -------------------------------------------------------------------
> List admin: http://cool.haxx.se/list/listinfo/curl-library
> Etiquette: http://curl.haxx.se/mail/etiquette.html
-------------------------------------------------------------------
List admin: http://cool.haxx.se/list/listinfo/curl-library
Etiquette: http://curl.haxx.se/mail/etiquette.html
Received on 2013-02-22