Bug 2962 - const in iconv arg 2
Summary: const in iconv arg 2
Status: RESOLVED INVALID
Alias: None
Product: glibc
Classification: Unclassified
Component: libc (show other bugs)
Version: unspecified
: P2 normal
Target Milestone: ---
Assignee: Ulrich Drepper
URL:
Keywords:
: 4085 (view as bug list)
Depends on:
Blocks:
 
Reported: 2006-07-25 19:03 UTC by gin
Modified: 2015-01-29 13:29 UTC (History)
4 users (show)

See Also:
Host:
Target:
Build:
Last reconfirmed:
fweimer: security-


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description gin 2006-07-25 19:03:01 UTC
Versions: all known, from 2.2.4 to 2.4.  This is why leaving version
unspecified.

`iconv' arg 2 is documented and declared in `iconv.h' to be `char
**__restrict', not `const char **' as
<http://www.opengroup.org/onlinepubs/007908799/xsh/iconv.html>
specifies.  In addition to differing with this standard, it is also
incompatibility with other `iconv' implementation, including one in
gnu `libiconv' as configured by default when no other `iconv'
implementation is installed, which uses `const char **' in this case.
(It will cause build error if application is compiled with `-Werror'
or equivalent.)

Certainly can have applications configure themselves to adapt to such
incompatibilities.  And better not to put such a burden on everybody
using `iconv', at least when switching between just 2 implementations
from the same project.
Comment 1 Ulrich Drepper 2006-07-25 22:15:56 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.
Comment 2 gin 2006-07-26 20:05:12 UTC
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.
Comment 3 gin 2006-08-14 14:29:07 UTC
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 (&current->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.
Comment 4 Ulrich Drepper 2006-08-14 16:30:51 UTC
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.
Comment 5 gin 2006-08-15 16:29:05 UTC
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.
Comment 6 Ulrich Drepper 2007-02-23 08:04:34 UTC
*** Bug 4085 has been marked as a duplicate of this bug. ***
Comment 7 Jens-Wolfhard Schicke 2007-05-10 16:17:50 UTC
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).
Comment 8 Ulrich Drepper 2007-05-10 17:44:33 UTC
Stop reopening this bug.  There is no problem.  If other implementations don't
agree they are at fault.
Comment 9 Alexander Petrossian (PAF) 2014-04-17 11:25:15 UTC
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 :)
Comment 10 Alexander Petrossian (PAF) 2014-04-17 12:10:09 UTC
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
Comment 11 Alexander Petrossian (PAF) 2014-04-23 15:18:33 UTC
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.