RE: Porting to OS/400: 4th patch release
Date: Thu, 23 Aug 2007 16:11:28 +0200
> Ah, I see. Maybe the curl API documentation should be changed to force users to set CONTENTSLENGTH before COPYCONTENTS, or, at least, make this a restriction of the OS/400 port. It would be nice to have a comment in the code explaining this, too.
Comment added. However, IMHO this not a problem of restriction, etc., but rather a matter of calling interface and semantics definition of curl_formadd() itself. Probably needs some system-independent additional doc.
> > > Probably a good idea to put an assert on CURLOPT_LASTENTRY on entry
I prefer to do the test on STRING_LAST: it changes only when a new option may require conversion. I added such a run-time check: an stderr message is output once.
> I put the thread buffer code and anything that depended on it into a curl- specific category, whereas the rest seemed pretty generic. It's a minor point.
I still resist ;-) Apart from entity names (which tend to respect curl standards), thread buffer code is part of OS400 wrappers because SSL and LDAP wrappers use it. In any case, someone who wants to use these in another project has to adapt this file, mainly because of the curl-specific names. Moving this code to a new specific project to enrich QADRT would be realistic in an environment "à la Linux", because package dependency hierarchy is the rule. This is not the case in the OS/400 world, at least for non-IBM packages. I even ported zlib to OS/400 some years ago, but I wont use it in curl, since OS/400 people are not used to deal with such a dependency!.
> One other thing would be good to add: appropriate #ifdef USE_QSOSSL, CURL_DISABLE_LDAP, and HAVE_GSSAPI blocks around the relevant sections in os400sys.c to allow compiling those options out if desired.
> I also spotted a couple more places where ICONV_MAX_EXPANSION should be
used: in Curl_SSL_Strerror_a and Curl_ldap_err2string_a.
ICONV_MAX_EXPANSION renamed as MAX_CONV_EXPANSION (because not iconv() dependent) and moved to os400sys.h. Used now in Curl_SSL_Strerror_a and Curl_ldap_err2string_a.
Thanks again for your remarks.
I'll commit it in a few minutes. Announce in a new thread.
Received on 2007-08-23