Summary: | const in iconv arg 2 | ||
---|---|---|---|
Product: | glibc | Reporter: | gin |
Component: | libc | Assignee: | Ulrich Drepper <drepper.fsp> |
Status: | RESOLVED INVALID | ||
Severity: | normal | CC: | alexander.petrossian, drahflow, glibc-bugs, john |
Priority: | P2 | Flags: | fweimer:
security-
|
Version: | unspecified | ||
Target Milestone: | --- | ||
Host: | Target: | ||
Build: | Last reconfirmed: |
Description
gin
2006-07-25 19:03:01 UTC
If you would have spent only a minute researching this you would have seen that this is a problem in the specification. Stop reporting such nonsense. Subject: Re: const in iconv arg 2 > this is a problem in the specification. Then not saying so explicitly is a bug in manual, and reporting it in <http://sourceware.org/bugzilla/show_bug.cgi?id=2963>. > If you would have spent only a minute researching this Until this bug is fixed, all of glibc (`iconv' implementation) users have to do so. I think most cost-benefit analyses would show that this is a rather inefficient use of resources. For this reason alone it is worth to be documented, whatever it is. > Stop reporting such nonsense. Don't dictate. Subject: Re: const in iconv arg 2 * Patch of one old version. > this is a problem in the specification. Easily changed at least one glibc version to conform to that specification as follows. Built for i686 from `glibc-2.3.3-12.8.100mdk' source rpm. (Compiler is `gcc-3.3.2-6mdk'.) Applied the following patch to sources from that rpm. Change `iconv' arg 2 type as specified in <http://www.opengroup.org/onlinepubs/007908799/xsh/iconv.html> * iconv/iconv.c (iconv): Do so in definition. * iconv/iconv.h (iconv): Do so in declaration. * iconv/iconv_charmap.c (use_from_charmap, use_to_charmap): * iconv/iconv_prog.c (process_block): * iconv/tst-iconv1.c (main): * iconv/tst-iconv2.c (main): * iconv/tst-iconv3.c (main): * iconvdata/bug-iconv1.c (main): * iconvdata/bug-iconv2.c (main): * iconvdata/tst-loading.c (main): * iconvdata/tst-e2big.c (test): * iconvdata/tst-iconv4.c (do_test): * iconvdata/bug-iconv4.c (xiconv): * iconvdata/tst-table-from.c (try): * iconvdata/tst-table-to.c (main): * catgets/gencat.c (read_input_file, normalize_line, open_conversion): Do so in calls. * manual/charset.texi (Generic Conversion Interface): Document. --- iconv/iconv.c +++ iconv/iconv.c @@ -29,7 +29,7 @@ size_t -iconv (iconv_t cd, char **inbuf, size_t *inbytesleft, char **outbuf, +iconv (iconv_t cd, const char **inbuf, size_t *inbytesleft, char **outbuf, size_t *outbytesleft) { __gconv_t gcd = (__gconv_t) cd; --- iconv/iconv.h +++ iconv/iconv.h @@ -40,7 +40,7 @@ /* Convert at most *INBYTESLEFT bytes from *INBUF according to the code conversion algorithm specified by CD and place up to *OUTBYTESLEFT bytes in buffer at *OUTBUF. */ -extern size_t iconv (iconv_t __cd, char **__restrict __inbuf, +extern size_t iconv (iconv_t __cd, const char **__restrict __inbuf, size_t *__restrict __inbytesleft, char **__restrict __outbuf, size_t *__restrict __outbytesleft); --- iconv/iconv_charmap.c 2001-12-29 18:57:13 +0300 +++ iconv/iconv_charmap.c @@ -287,7 +287,7 @@ /* There is a chance. Try the iconv module. */ wchar_t inbuf[1] = { in->ucs4 }; unsigned char outbuf[64]; - char *inptr = (char *) inbuf; + const char *inptr = (const char *) inbuf; size_t inlen = sizeof (inbuf); char *outptr = (char *) outbuf; size_t outlen = sizeof (outbuf); @@ -357,7 +357,7 @@ /* There is a chance. Try the iconv module. */ wchar_t inbuf[1] = { out->ucs4 }; unsigned char outbuf[64]; - char *inptr = (char *) inbuf; + char *inptr = (const char *) inbuf; size_t inlen = sizeof (inbuf); char *outptr = (char *) outbuf; size_t outlen = sizeof (outbuf); --- iconv/iconv_prog.c 2003-04-14 17:09:19.000000000 +0400 +++ iconv/iconv_prog.c 2006-08-13 02:51:44.551436008 +0400 @@ -109,7 +109,8 @@ int omit_invalid; /* Prototypes for the functions doing the actual work. */ -static int process_block (iconv_t cd, char *addr, size_t len, FILE *output); +static int process_block +(iconv_t cd, const char *addr, size_t len, FILE *output); static int process_fd (iconv_t cd, int fd, FILE *output); static int process_file (iconv_t cd, FILE *input, FILE *output); static void print_known_names (void) internal_function; @@ -430,7 +431,7 @@ static int -process_block (iconv_t cd, char *addr, size_t len, FILE *output) +process_block (iconv_t cd, const char *addr, size_t len, FILE *output) { #define OUTBUF_SIZE 32768 const char *start = addr; --- iconv/tst-iconv1.c 2001-09-01 23:21:50.000000000 +0400 +++ iconv/tst-iconv1.c 2006-08-13 03:28:27.709505072 +0400 @@ -11,7 +11,7 @@ char utf8[5]; wchar_t ucs4[5]; iconv_t cd; - char *inbuf; + const char *inbuf; char *outbuf; size_t inbytes; size_t outbytes; --- iconv/tst-iconv2.c 2001-07-06 08:54:47.000000000 +0400 +++ iconv/tst-iconv2.c 2006-08-13 03:29:42.331160856 +0400 @@ -32,7 +32,7 @@ char buf[3]; const wchar_t wc[1] = L"a"; iconv_t cd; - char *inptr; + const char *inptr; size_t inlen; char *outptr; size_t outlen; @@ -49,7 +49,7 @@ exit (1); } - inptr = (char *) wc; + inptr = (const char *) wc; inlen = sizeof (wchar_t); outptr = buf; outlen = 3; @@ -70,7 +70,7 @@ result = 1; } - if (inptr != (char *) wc) + if (inptr != (const char *) wc) { puts ("inptr changed"); result = 1; --- iconv/tst-iconv3.c 2003-08-02 10:35:08.000000000 +0400 +++ iconv/tst-iconv3.c 2006-08-13 03:30:33.591368120 +0400 @@ -15,7 +15,7 @@ iconv_t cd; int i; - char *inptr; + const char *inptr; char *outptr; size_t inbytes_left, outbytes_left; int count; --- iconvdata/bug-iconv1.c 2000-08-23 09:50:09.000000000 +0400 +++ iconvdata/bug-iconv1.c 2006-08-13 03:32:06.357265560 +0400 @@ -14,7 +14,7 @@ size_t outbufsize = 10; /* 10 is too small to store full result (intentional) */ size_t inleft, outleft; - char *in_p = (char *) in; + const char *in_p = (const char *) in; char out[outbufsize]; char *out_p = out; iconv_t cd; --- iconvdata/bug-iconv2.c 2000-11-20 20:38:06.000000000 +0300 +++ iconvdata/bug-iconv2.c 2006-08-13 03:32:48.862803736 +0400 @@ -15,7 +15,7 @@ iconv_t dummy_cd[8], cd_a; int i; char buffer[1024], *to = buffer; - char *from = (char *) "foobar"; + const char *from = (const char *) "foobar"; size_t to_left = 1024, from_left = 6; /* load dummy modules */ --- iconvdata/tst-loading.c 2002-09-24 08:21:09.000000000 +0400 +++ iconvdata/tst-loading.c 2006-08-13 03:33:28.623759152 +0400 @@ -133,7 +133,7 @@ if (modules[idx].state == unloaded) { char outbuf[10000]; - char *inptr = (char *) inbuf; + const char *inptr = (const char *) inbuf; size_t insize = sizeof (inbuf) - 1; char *outptr = outbuf; size_t outsize = sizeof (outbuf); --- iconvdata/tst-e2big.c 2002-09-30 10:53:25.000000000 +0400 +++ iconvdata/tst-e2big.c 2006-08-13 03:34:16.348503888 +0400 @@ -35,7 +35,7 @@ { char *outbuf = alloca (outbufsize); iconv_t cd; - char *inptr; + const char *inptr; size_t inlen; char *outptr; size_t outlen; --- iconvdata/tst-iconv4.c 2002-11-26 05:18:56.000000000 +0300 +++ iconvdata/tst-iconv4.c 2006-08-13 03:35:16.766318984 +0400 @@ -16,7 +16,7 @@ } char instr[] = "a"; - char *inptr = instr; + const char *inptr = instr; size_t inlen = strlen (instr); char buf[200]; char *outptr = buf; --- iconvdata/bug-iconv4.c 2003-03-26 11:10:58.000000000 +0300 +++ iconvdata/bug-iconv4.c 2006-08-13 03:35:44.864047480 +0400 @@ -14,7 +14,7 @@ xiconv (iconv_t cd, int out_size) { unsigned char euc[4]; - char *inp = (char *) UCS_STR; + const char *inp = (const char *) UCS_STR; char *outp = euc; size_t inbytesleft = strlen (UCS_STR); size_t outbytesleft = out_size; --- iconvdata/tst-table-from.c 2002-09-24 07:43:30.000000000 +0400 +++ iconvdata/tst-table-from.c 2006-08-13 03:38:26.040544928 +0400 @@ -70,7 +70,7 @@ size_t result; iconv (cd, NULL, NULL, NULL, NULL); - result = iconv (cd, (char **) &inbuf, &inbytesleft, &outbuf, &outbytesleft); + result = iconv (cd, &inbuf, &inbytesleft, &outbuf, &outbytesleft); if (result != (size_t)(-1)) result = iconv (cd, NULL, NULL, &outbuf, &outbytesleft); --- iconvdata/tst-table-to.c 2002-05-15 09:41:25.000000000 +0400 +++ iconvdata/tst-table-to.c 2006-08-13 03:39:12.531477232 +0400 @@ -81,7 +81,7 @@ iconv (cd, NULL, NULL, NULL, NULL); result = iconv (cd, - (char **) &inbuf, &inbytesleft, + &inbuf, &inbytesleft, &outbuf, &outbytesleft); if (result != (size_t)(-1)) result2 = iconv (cd, NULL, NULL, &outbuf, &outbytesleft); --- catgets/gencat.c 2003-04-14 17:09:19.000000000 +0400 +++ catgets/gencat.c 2006-08-13 02:20:06.912920912 +0400 @@ -526,7 +526,7 @@ else if (strncmp (&this_line[1], "quote", 5) == 0) { char buf[2]; - char *bufptr; + const char *bufptr; size_t buflen; char *wbufptr; size_t wbuflen; @@ -686,7 +686,7 @@ if (message_number != 0) { - char *inbuf; + const char *inbuf; size_t inlen; char *outbuf; size_t outlen; @@ -755,7 +755,7 @@ /* Now fill in the new string. It should never happen that the replaced string is longer than the original. */ - inbuf = (char *) wbuf; + inbuf = (const char *) wbuf; inlen = (wcslen (wbuf) + 1) * sizeof (wchar_t); outlen = obstack_room (¤t->mem_pool); @@ -1165,7 +1165,7 @@ { int number; char cbuf[2]; - char *cbufptr; + const char *cbufptr; size_t cbufin; wchar_t wcbuf[2]; char *wcbufptr; @@ -1310,7 +1310,7 @@ wchar_t *escape_charp) { char buf[2]; - char *bufptr; + const char *bufptr; size_t bufsize; wchar_t wbuf[2]; char *wbufptr; --- manual/charset.texi 2003-07-24 13:19:52.000000000 +0400 +++ manual/charset.texi 2006-08-13 04:11:38.566634920 +0400 @@ -1707,7 +1707,7 @@ @comment iconv.h @comment XPG2 -@deftypefun size_t iconv (iconv_t @var{cd}, char **@var{inbuf}, size_t *@var{inbytesleft}, char **@var{outbuf}, size_t *@var{outbytesleft}) +@deftypefun size_t iconv (iconv_t @var{cd}, const char **@var{inbuf}, size_t *@var{inbytesleft}, char **@var{outbuf}, size_t *@var{outbytesleft}) @cindex stateful The @code{iconv} function converts the text in the input buffer according to the rules associated with the descriptor @var{cd} and It still rebuilt without `iconv' arg 2 type mismatch warnings. The problem, if once existed, definitely does not affect at least glibc installation as of this version. If it manifests at all, it does so in some complex way that is surely not detectable with <only a minute researching this>. Perhaps specification conforming coding style wrt calling `iconv' is inconvenient to some. This has nothing to do with specification itself. Certainly, given how long current declaration stayed, there is many code instances depending on it. Nothing terribly complex. Again, nothing to do with published specification. Original `inp' initialization in `iconvdata/bug-iconv4.c' even disregards string constants being read- only in modern compilers. (It does not get `-Wwrite-strings' warning, at least with some gcc versions.) * Patch for current glibc version? Creating and trying one requires building that version. And <http://sourceware.org/bugzilla/show_bug.cgi?id=333> strongly discourages doing so, by saying: > Building glibc properly is a complex operation with many particular > dependencies. If you insist on doing it yourself, you will have to > investigate your issues and resolve them yourself. essentially advising everybody to confine themselves to > the binary packages made by your operating system distributor In my case they are as above. Other criticisms of this attitude are already found in comments of that bug. What are you talking about? That patch is completely, utterly wrong. The current code and the manual is correct, there is no problem. The current spec has already been revised with an interpretation request and the first draft of the upcoming spec is fixed. There is no const. Period. Your patch makes the glibc of whatever moron is using it non-compliant. The only possible causes for problem are stupid comments like this which are added despite the fact that I previously already said that everything as it is is correct. Stop pollution bz. Subject: Re: const in iconv arg 2 With <http://www.opengroup.org/onlinepubs/009695399/functions/iconv.html> being widely and *anonymously* available, I confirm this issue to be closed. With having <interpretation request> key phrase for search finding it was truly <only a minute>, chasing a couple of links. Without knowing these keywords finding was hard to impossible. And such a knowledge is part of fairly specific details of standardization procedures. > despite the fact that I previously already said that everything as > it is is correct. I understand what one feels in this case. Perhaps the place where it was said are some fora like this. Then updating standard references in manual as suggested by <http://sourceware.org/bugzilla/show_bug.cgi?id=2963> may be more effective. Admitting that I posted noise. And *not* apologizing for this. Figuring that it was noise was not trivial. Admitting that generally I am reluctant to register with bodies like opengroup, and this was a partial cause of my error. Will describe the details on request. *** Bug 4085 has been marked as a duplicate of this bug. *** Ok, I read different opengroup standards, some stating the second parameter should be const char **, others telling it should be char **. The current libiconv documentation (http://www.gnu.org/software/libiconv/documentation/libiconv/iconv.3.html) says, it is const char **. The _code_ uses it in the following fashion (only basically): result = __gconv (gcd, (const unsigned char **) inbuf, (const unsigned char *) (*inbuf + *inbytesleft), (unsigned char **) outbuf, (unsigned char *) (*outbuf + *outbytesleft), &irreversible); All other uses, are for checking == NULL... http://sources.redhat.com/cgi-bin/cvsweb.cgi/libc/iconv/iconv.c?rev=1.15&content-type=text/x-cvsweb-markup&cvsroot=glibc After all a type is not just some fancy word, it is a contract between the iconv function and it's user. And the contract includes (I assume this only, but every documentation I read so far implies it) that iconv will _not_ change the inbuf contents. So it should be const (once more). I mean, just because some standard says it's char **, it is not any more right. Either the function changes the values, then it's char **, or it does not then it's const char **. To users of the function this is especially unfriendly, because they either need to cast their constant data (which is not guaranteed to work out well), or need to copy it first (which is slow). Stop reopening this bug. There is no problem. If other implementations don't agree they are at fault. Gin, please share what you've found with those keywords? My guts are against such strange standard and they feel like learning "why they decided to do such unnatural thing as declaring inbuf not const". Ulrich, please thank you for your work. Please borrow some patience somewhere :) Found http://www.opengroup.org/infosrv/XPG4_Brand/resolutions/XoTGBase94-001.ps.Z Quote: Since applications should be able to pass an argument of type char ** to iconv(), iconv() page should be corrected. ------- void iconv(const char** inbuf); void f() { char* inbuf; iconv(&inbuf); } ------ cc t.c t.c: In function ‘f’: t.c:5: warning: passing argument 1 of ‘iconv’ from incompatible pointer type For history: found why: http://c-faq.com/ansi/constmismatch.html in short: be such conversion possible, theoretically, icon() could have moved client pointer to point to some really_const_memory, while client could, very theoretically, write there without any warnings but with possibly sad consequences to users of really_const_memory. |