Daniel Veillard wrote:
as promised I'm back on the issue, took a while but heh, we waited 10+
years since first exchange about it :-) If it's like for wine, it couldn't be better !
http://tomayko.com/writings/the-thing-about-git which show where git helps you in such situation :-)
Nice tutorial. But the reasons why I sent it in a single patch were 1) Publish something usable on OS400 quickly. 2) Let you have a general sight and not too much handling to get/apply it.
- a patch to encoding.c about using (iconv_t) -1 instead of
comparing to NULL, I can understand why you do it but I would like to know first if we could use (iconv_t) 0 instead, but reading the README.OS400 I'm not sure it's possible. Because of the OS400 architecture (16-byte structured pointer, aligned + an additional bit (non-addressable) to tell if the pointer is valid), the iconv native implementation is not standard at all: - Character set names are all of the form "IBMCCSIDnnnnn..." (EBCDIC) where nnnnn is a 5-digit decimal caracter code identifier. - iconv_t is a structure (not a pointer). So you cannot even comprare it to (iconv_t) -1 or NULL (validity can be checked using a field of the structure). As a consequence, I had to write a wrapper, performing the name conversion and mapping iconv_t to some scalar type. Since casting an integer to a pointer is not officially supported, I started by using "typedef int iconv_t" and a mapping array. This explains why I've changed NULL to (iconv_t) -1 at first: this respected the POSIX standards. But this solution is too heavy for multithreaded environments and I noticed that, although not officially supported, casting -1 to a pointer does work, at least for equality comparison. So I moved to a pointer iconv_t, but did not revert the encoding.c changes. As a consequence, OS400 code should work without changing encoding.c, at least for the current versions of the OS. I presume you prefer (iconv_t) 0 for initialization purposes, right ? In fact we have to compare the 3 alternatives: 1) Current (NULL): implies iconv_t is a pointer. 2) (iconv_t) 0: implies iconv_t is a scalar. Seems POSIX-compliant, but will reject "conversion descriptors" == 0 even if iconv_t is an integer: very bad. 3) (iconv_t) -1: 100% POSIX-compliant. Altough possible for OS400, I don't see a real improvement moving to 2). I would then prefer to stay with 1), with the reserve of changing my mind if future OS400 versions will fail on casting integers to pointers.
Of all the patches applying to existing code, I applied most of them,
you can find them in git HEAD upstream. Many thanks for that.
I dropped the patch for NEWS as it's a generated file and will be
overwritten on next commit, since those are docs I assume it's not critical anyway. As you say, not critical, but I'd like to copy this file to somewhere where line lengths have to be < 100, so I need to change it. Please find a patch in attachment: it changes the NEWS file, but also the doc/news.html in a way that does not (erroneously) concatenate lines when producing NEWS and does not alterate real html rendering.
Then there is the os400 directory, ahum, that's huge ....
Agreed, yes! At the first level, it contains compilation scripts, EBCDIC/ASCII wrappers and supplementary encoding support. Subdirectories contain: - dlfcn emulation - iconv wrapper and a program to rebuild the character set name mapping tables. - libxmlrpg. It's not debugging output, as you think, but a handmade translation of all include files to support the ILE/RPG programming language. Very few people use C/C++ on OS400 and the top compiled language (in term of use!) is ILE/RPG. If we really want to make libxml2 accessible to OS400 programmers, ILE/RPG support is mandatory. In fact, binding for this language consists in these files for about 98% and only 2% executable support code. Please also note that the real translation from C header files cannot be made accurately by a conversion program (or else I had written it a long time ago :-)
having an author for the copyright would be a required change, same
for all the files in the new directory :-) I'll set it for the next patches.
Please send incremental patches against your existing libxml2.os400
tree to avoid having to regenerate everything :-) You mean mine (incl the big patch)? Or the current git head? Many thanks for your time and attention. Regards, Patrick
Attachment:
news.patch
Description: news.patch