On Tue, 13 Jan 2009, Markus Moeller wrote:

> I think I fixed the points above.

Ok, I edited in a few further changes I think it needed:

- Applied and synced with current CVS.

- I removed all uses of sprintf() and included the correct header for our
   internal printf stuff. I think a few places could use aprintf() instead of
   malloc() and memcpy()s etc, but...

- I also unified a few things between the two implementations, since I don't
   think a lot of code needs to bother about the diffs. Thus they both provide
   the same function name now.

- I modded some code to put the string
   data->set.str[STRING_SOCKS5_GSSAPI_SERVICE] into a local variable as
   repeated accesses to that long name makes the code awkward to read!

(Please verify that everything still works!)

But most importantly what is still left to fix:

  * I added checks for the malloc() calls I could find with returns on failure.
    These are not really fine since they will leak memory in case of failure.
    Adding proper ways to bail out may require use of a few more functions than
    currently.

  * I'm not friends with large stack uses and 64K matches that description imo.
    I would like that memory to get allocated. Then the pointer to the buffer
    is passed on to the sub function with a pointer only - without a length,
    and the Curl_SOCKS5_gssapi_negotiate() implementations just assume
    sufficient length. That's asking a little too much for trouble in the
    future. I would like to see a length argument being passed in to the
    function as well, and the function of course make really sure that it never
    writes beyond that boundary.

Would you rather like me to commit the work we have right now and you can base
further patches on that, or do you rather polish the complete patch like this?

-- 
  / daniel.haxx.se