regression caused by fix of bug #13691
Bruno Haible
bruno@clisp.org
Sun May 13 23:41:00 GMT 2012
Hi,
The "fix" of BZ #13691 committed on 2012-04-02
<http://sourceware.org/bugzilla/show_bug.cgi?id=13691>
<http://sourceware.org/ml/libc-alpha/2012-03/msg01110.html>
<http://sourceware.org/ml/libc-alpha/2012-04/msg00039.html>
<http://sourceware.org/git/?p=glibc.git;a=commitdiff;h=39c59c35723120c32dc42dde4115bba92305179f>
is broken: it introduces a regression in the iconv() function for
this encoding.
Facts to remember when pushing changes into iconvdata/*:
* The iconvdata/* converters are used by the gconv_* functions, which
are internal to glibc and used by
1) iconv(),
2) the wcsmbs/* directory: wide string conversion functins.
* The converters support feeding the input and pulling out the output
in chunks, as this is needed for iconv(). The converters are optimized
for large chunks, and for getting all input in a single chunk. But
in the worst case they can get their input 1 byte at a time, and
the output must be the same as when all the input it fed at once.
========================== regression.c ==========================
#include <iconv.h>
#include <stdio.h>
#include <stdlib.h>
#define ASSERT(expr) \
do \
{ \
if (!(expr)) \
{ \
fprintf (stderr, "%s:%d: assertion failed\n", \
__FILE__, __LINE__); \
fflush (stderr); \
abort (); \
} \
} \
while (0)
int
main ()
{
/* Test that 0x41 0xB2 = U+0041 U+0303 combines to U+00C3. */
{
iconv_t cd = iconv_open ("UTF-8", "TCVN-5712");
ASSERT (cd != (iconv_t) -1);
{
char in[2] = { (char) 0x41, (char) 0xB2 };
char out[10];
char *inptr = in;
size_t inbufsize = sizeof (in);
char *outptr = out;
size_t outbufsize = sizeof (out);
size_t ret = iconv (cd, &inptr, &inbufsize, &outptr, &outbufsize);
ASSERT (ret == 0);
ASSERT (inptr == in + 2);
ASSERT (inbufsize == sizeof (in) - 2);
ASSERT (outptr == out + 2);
ASSERT (outbufsize == sizeof (out) - 2);
ASSERT (out[0] == (char) 0xC3 && out[1] == (char) 0x83);
}
iconv_close (cd);
}
/* Likewise, in two steps. */
{
iconv_t cd = iconv_open ("UTF-8", "TCVN-5712");
ASSERT (cd != (iconv_t) -1);
{
char in1[1] = { (char) 0x41 };
char in2[1] = { (char) 0xB2 };
char out[10];
char *inptr = in1;
size_t inbufsize = sizeof (in1);
char *outptr = out;
size_t outbufsize = sizeof (out);
size_t ret = iconv (cd, &inptr, &inbufsize, &outptr, &outbufsize);
ASSERT (ret == 0);
ASSERT (inptr == in1 + 1);
ASSERT (inbufsize == sizeof (in1) - 1);
ASSERT (outptr == out);
ASSERT (outbufsize == sizeof (out));
inptr = in2;
inbufsize = sizeof (in2);
ret = iconv (cd, &inptr, &inbufsize, &outptr, &outbufsize);
ASSERT (ret == 0);
ASSERT (inptr == in2 + 1);
ASSERT (inbufsize == sizeof (in2) - 1);
ASSERT (outptr == out + 2);
ASSERT (outbufsize == sizeof (out) - 2);
ASSERT (out[0] == (char) 0xC3 && out[1] == (char) 0x83);
}
iconv_close (cd);
}
return 0;
}
========================================================================
Before Tulio's "fix":
$ ./regression ; echo $?
0
After Tulio's "fix":
$ ./regression ; echo $?
regression.c:58: assertion failed
Aborted
134
Tulio's "fix" is also totally besides the point because he has not
understood the issue.
The issue is that when the gconv, iconv and wcsmbs functionality was
added to glibc, the assumption was that wcsmbs needs to support only
stateless encodings.
Whereas TCVN-5712 and CP1258 are stateful encodings. You can recognize
stateful encodings in glibc by the fact that they define a nontrivial
EMIT_SHIFT_TO_INIT macro.
But localedata/SUPPORTED lists vi_VN.TCVN/TCVN5712-1 as a supported
locale encoding. Although this is rather academic. The number of users
that use this locale nowadays is likely smaller than 1.
There are two possible fixes:
1) Back out Tulio's changes to iconvdata/ and remove vi_VN.TCVN/TCVN5712-1
from localedata/SUPPORTED.
2) Back out Tulio's changes to iconvdata/ and add support in wcsmbs/* for
stateful encodings.
What would it take to go for option 2? Here are test programs for
mbrtowc(), mbsrtowcs(), mbsnrtowcs().
=======================================================================
#include <locale.h>
#include <stdlib.h>
#include <stdio.h>
#include <string.h>
#include <wchar.h>
int main ()
{
mbstate_t ps;
wchar_t wc;
size_t ret;
setlocale (LC_ALL, "vi_VN.TCVN");
memset (&ps, '\0', sizeof (ps));
wc = 0xdddd;
ret = mbrtowc (&wc, "A", 1, &ps);
printf ("ret = %zu, wc = %x\n", ret, wc);
return 0;
}
=======================================================================
Result:
ret = 1, wc = dddd
This is wrong: mbrtowc cannot return 1 but not set the output wide
character.
=======================================================================
#include <locale.h>
#include <stdlib.h>
#include <stdio.h>
#include <string.h>
#include <wchar.h>
int main ()
{
mbstate_t ps;
const char *src = "A";
wchar_t wc[5];
size_t ret;
setlocale (LC_ALL, "vi_VN.TCVN");
memset (&ps, '\0', sizeof (ps));
wc[0] = 0xdddd;
wc[1] = 0xeeee;
ret = mbsrtowcs (wc, &src, 1, &ps);
printf ("ret = %zu, wc = %x %x\n", ret, wc[0], wc[1]);
return 0;
}
=======================================================================
Result:
ret = 1, wc = 41 eeee
This is OK.
=======================================================================
#include <locale.h>
#include <stdlib.h>
#include <stdio.h>
#include <string.h>
#include <wchar.h>
int main ()
{
mbstate_t ps;
const char *src = "A";
const char *tmp = src;
wchar_t wc[5];
size_t ret;
setlocale (LC_ALL, "vi_VN.TCVN");
memset (&ps, '\0', sizeof (ps));
wc[0] = 0xdddd;
wc[1] = 0xeeee;
ret = mbsnrtowcs (wc, &tmp, src + 1 - tmp, 1, &ps);
printf ("ret = %zu, wc = %x %x\n", ret, wc[0], wc[1]);
return 0;
}
=======================================================================
Result:
a.out: mbsnrtowcs.c:123: __mbsnrtowcs: Assertion `result > 0' failed.
This is wrong and was the subject of BZ #13691.
Most programs uses either mbrtowc or mbstowcs to convert strings to
wide-char encoding. Few programs use mbsnrtowcs. Therefore *if* someone
wants to support stateful encodings in wcsmbs/*, the most important
place to fix is the mbrtowc() behaviour. But this is also the most
difficult one. I cannot see how to make the following requirements
coexist:
* mbrtowc(&wc, "A", 1, &ps) shall set wc = L'A'.
* mbrtowc(&wc, "A\xb0", 2, &ps) shall set wc = 0x00C0
(LATIN CAPITAL LETTER A WITH GRAVE)
* mbrtowc can be used to process a string byte for byte; it returns -2
when a byte sequence is incomplete. In particular this means that the
sequence of calls
mbrtowc(&wc, "A", 1, &ps) => -2
mbrtowc(&wc, "\xb0", 1, &ps) => 1, wc = 0x00C0
produces an intermediate -2 without setting wc and then sets wc in the
second call.
The problem is that when a string like "CBA" is being processed byte for byte,
we want to see a wc = L'A' come out at the end. But there is no way for the
programmer to tell the mbrtowc() function that the string is terminated.
In other words, mbrtowc() assumes that the multibyte sequence of an entire
wide character does not start with the multibyte sequence for another wide
character. The TCVN-5712 and CP1258 encodings don't have this property.
So, I don't see a way to implement option 2.
This also means that there are likely zero users of the vi_VN.TCVN locale,
because many programs would misbehave in this locale.
I therefore propose to
- revert Tulio's "fix",
- remove vi_VN.TCVN/TCVN5712-1
- from localedata/SUPPORTED, and resolve BZ #13691 as "Won't fix".
Patch attached.
Bruno
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-BZ-13691-Revert-breakage-of-iconv-converter-for-TCVN.patch
Type: text/x-patch
Size: 2027 bytes
Desc: not available
URL: <http://sourceware.org/pipermail/libc-alpha/attachments/20120513/b9668da7/attachment.bin>
More information about the Libc-alpha
mailing list