curl-library
RE: Porting to OS/400: 3rd patch release
Date: Tue, 14 Aug 2007 15:14:32 +0200
Many thanks for your code review, Dan.
> The config.h files tradionally hold simple boolean #defines. Anything
more elaborate should probably go into setup.h (or setup-os400.h).
I agree. I just not want to be too intrusive in the main tree. If an
additional OS400-specific file in the lib subdir is not too much for
anybody, I can change it that way. Anybody's opinion ?
> The buffer assumptions in makeOS400IconvCode() should be documented to
reduce the chance that someone will misuse them and cause a buffer
overflow.
The iconv implementation on OS400 is NOT portable and the code can
hardly be reused for another platform. The OS-specific iconv identifiers
are heavily described by IBM, but yes, I can add a comment.
> It should be documented that the OS/400 support is not thread safe
(static buffers, et. al.). However os400sys.c uses POSIX threads calls?
> What's the purpose?
The standard libcurl API IS threadsafe: for the moment, POSIX threads
calls guarantee threadsafeness behavior of wrappers between libcurl and
OS, but not for non-standard EBCDIC API. This is planned in a near
future.
> The list of tests in make-tests.sh is already out of date and missing
tests. It looks like this is now the only case where the OS/400 build
system duplicates information from the normal curl build system, which
is good.
'will try to remove this duplication too, but the shell script to do
this (by reading Makefile.am) might be a bit complicated...
Thanks again for your comments
Patrick
Received on 2007-08-14