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
news.patch
Description: news.patch
_______________________________________________ xml mailing list, project page http://xmlsoft.org/ xml@gnome.org https://mail.gnome.org/mailman/listinfo/xml
