Georg Baum wrote:
Am Freitag, 29. Dezember 2006 19:54 schrieb Abdelrazak Younes:
Georg Baum wrote:
You will probably get some from Lars, but I have one of my own: I don't
like the replacement of iconv_t by int and the resulting casting.
Please
revert that. The casting of -1 to iconv_t is safer. Do you really know
that int will always be compatible to iconv_t?
I am not changing the semantics at all. The casting happens in any case,
be it internally or externally to the iconv_convert function. I just
factorized the number of casting, that's all. The result is identical to
the old code.
No. You changed some variables of type iconv_t to type int. That might
result in identical code on your platform, but nobody knows whether that
is the same on all other platforms. What I mean is that it is safer to
cast -1 to iconv_t, and that all variables that store conversion
descriptors should be of type iconv_t (as it was before). And if you don't
like the many casts of of -1 you could add a
const iconv_t invalid_cd = (iconv_t)-1;
and use it. Then you have only one cast.
OK. I'll see what I can do.
Besides, this static variable is declared all other the place is not
used anywhere except within iconv_convert so I really don't see why we
keep it.
Which static variable do you mean?
This invalid_cd is declared static in all the conversion function:
static int cd = -1;
But is not used at all except that it is tested at the beginning of
iconv_convert().
IMO an important bit is missing: Completing a conversion that is
incomplete
because the buffer was too small.
Read more closely the code, I don't need a fixed size buffer any more. I
am writing the converted data right into the docstring output that has
already enough place.
Sorry, I was unclear. That has nothing to with your changes but was a
general comment (and I did not mean that _you_ should fix that problem):
The static buffer ist still there in the other iconv_convert.
Yes but I don't use that in my new utf8_to_ucs4() in docstring.[Ch] and
I propose that we do the same for conversion.
We cannot always increase the bufffer if
that happens, so IMO we need to do that for 1.5.0, because users will
otherwise get silent data corruption.
As said above, we don't need to do that anymore with my version.
And why is it still there then?
Because I modified only the utf8_to_ucs4 not all the other conversions.
We don't use anymore the template version of iconv_convert() which
contains the fixed size buffer.
With that in mind I do not like that you make iconv_convert a part of
the
public interface. It has an ugly interface that should be kept private.
Come one, it is only used in docstring. I can remove it from the
unicode.h and declare it as extern in docstring.C if you prefer.
That would be worse IMO.
Then my poposal in my other mail (move that to docstring.C) would fits you?
Abdel.
