On Fri, 10 Nov 2000, Mike Nordell wrote:
> Vlad Harchev wrote:
> > On Thu, 9 Nov 2000, Mike Nordell wrote:
> >
> > > iconv(iconv_handle,NULL,NULL,NULL,NULL);
> >
> > Such usage means "reset conversion state" (i.e. reset shift state). Any
> > iconv implementation should support it.
>
> This "stub" implementation (from the wv snapshot) don't. It crashes. I now
Wow! Now wv has iconv stub too! (Expat has iconv stub before, but they seem
to be moved to wv).
> know the "fault" is mine, since I should have remembered to use the _real_
> iconv, but then I fail to see why this "stub" implementation even exists in
> our wv (snapshot), since it just crashes. I would have found out about the
> errors of my ways much faster if I got a linker error. Any thoughts?
>
> > Probably conv_handle was ((iconv_t)-1) - that's why it could crash.
>
> That's one of the errors.
>
> > Please double check that iconv implementation doesn't come from expat -
> > it has a very cutdown version of iconv that knows very little.
> > It's definitely not compiled on unix. Dunno for win32.
>
> I used it on Win32, and it won't work. Actually, I'd be surprised if it
> worked on any platform why my first impression is that it should be removed.
> I could be wrong though, we might want those friendly SEGVs and asserts,
> compared to an unfriendly linker error stating we left a lib out. :-)
>
> Now on to the current Win32 things.
> As noted earlier, the spell checker seems to fail. Since I'm using Win32, I
> used spell/newMain.c instead of pspell.c.
>
> Findings:
> The call to iconv() from SpellCheckNWord16 is not checked for its return
> value. This is bad, since I got it to fail (returning -1).
Nice idea, but if it fails, AW is totally screwed up (cut&paste won't work,
etc) - so probably it won't be wise to keep it from crashing that much :)
>
> SpellCheckNWord16 (spell/newMain.c) should probably take a wchar_t pointer,
> and not an unsigned short*. AFAIK input must be in host byte order otherwise
> the "manual" truncating copy in case "translate_in" is invalid will fail.
I think "UCS2" means two-byte unicode values. Short is 2 bytes on majority of
platforms, so it's OK to use it.
As for wchar_t - God knows what it is on various platforms (not necessary
'short' I mean).
> Looking in iconv() (iconv.c) I find
> incount = cd->ifuncs.xxx_mbtowc(cd,&wc,inptr,inleft);
>
> This becomes bad, since inptr is now in iconv-native byte order (i.e. big
> endian), but cd->ifuncs.xxx_mbtowc points to the function
> ucs2internal_mbtowc() (ucs2internal.h) which outputs in host-byte-order (a
> wchar_t is always in native byte order).
> Perhaps this function really is to give output in big-endian order, but then
> it is an error to use wchar_t for output.
> What makes it even more buggy is that iconv() later calls
> cd->ofuncs.xxx_wctomb which becomes iso8859_1_wctomb. Since this function
> assumes a wchar_t (host-order) but gets a iconv-UCS-2 (big-endian). I don't
> think I need to explain the implications.
>
> Whatever, I gave up on it for the moment. I think I have to do some catching
> up to be able to give some more educated
>
> Why do I feel that this is just a tip of an iceberg?
I don't recommend you to look at the iconv's sources.
>
> And finally another small thing. It seems some have forgotten the
> indentation rules, tab=4, indent is one tab or four spaces. :-)
I have an impression that someone doesn't care about indentation because of
the lack of time (and he fills very sorry about that) :)
>
> /Mike - please don't cc
>
Best regards,
-Vlad
