Re: [Bug-gnulib] getline & getline_safe

[Derek Robert Price]

Bruno Haible wrote:

> Derek Robert Price wrote:
>  
>
> > I've merged your recent changes back into my
> > patch so it is considerably smaller.  Please comment.
> >    
>
> I've now committed part of it:
>
> 2003-07-24  Derek Robert Price  <address@hidden>
>            Bruno Haible  <address@hidden>
>
>        * getline.h (getline, getdelim): Change return type to ssize_t.
>        * getline.c (getline, getdelim): Likewise.
>        Remove _GNU_SOURCE define; now it's defined in config.h through
>        m4/getline.m4.
>
> I didn't commit the semantic change of getndelim2 because of the reasons
> explained in
> http://mail.gnu.org/archive/html/bug-gnulib/2003-07/msg00062.html
>  

And I still disagree for the reasons explained in: 
<http://mail.gnu.org/archive/html/bug-gnulib/2003-07/msg00064.html>.   
Basically, in my version the error case can be detected and dealt with.  
In yours the last byte is lost forever.  Your case is definately the 
more broken of the two.

As for "line-oriented protocols", I would assume the developer knew what 
they were doing when they made a call to "getnline" as opposed to getline.

Also please consider simplicity!  My patch looks complicated, but only 
because you added the memory bounding!  Before you commited that code, 
my patch only required five or six lines added to getndelim2.c to 
accomplish its chore!

You've also neglected to comment on the reordering of the arguments to 
getndelim2.  The new order is much more intuitive, similar to that of 
getdelim, getline, getndelim, and getnline, and now is the time to 
change it before many projects start using the freshly exported API.

Finally, the original bug reported by my user was a typing conflict 
which occurred on 64-bit systems where ssize_t was not the same thing as 
an int.  Now, I'm not sure how they managed to get past your $if 
__GLIBC__ < 2 on that system to hit the prototypes in getline.h, but 
they did.  I switched the header to switch on HAVE_WORKING_GETLINE & 
HAVE_GETDELIM as detected by configure (though I just read an autoconf 
thread from Paul Eggert that suggested we should use HAVE_DECL_* 
switches in most cases instead), and this fixed the problem.

The original report is here: 
<http://ccvs.cvshome.org/issues/show_bug.cgi?id=130>

The current state of my patch:

Index: lib/getline.c
===================================================================
RCS file: /cvsroot/gnulib/gnulib/lib/getline.c,v
retrieving revision 1.15
diff -u -r1.15 getline.c
--- lib/getline.c       24 Jul 2003 15:23:15 -0000      1.15
+++ lib/getline.c       24 Jul 2003 16:19:42 -0000
@@ -26,12 +26,6 @@
/* Specification.  */
#include "getline.h"

-#include <stddef.h>
-#include <stdio.h>
-
-/* Get ssize_t.  */
-#include <sys/types.h>
-
#if defined __GNU_LIBRARY__ && HAVE_GETDELIM

ssize_t
@@ -47,12 +41,14 @@
ssize_t
getline (char **lineptr, size_t *linesize, FILE *stream)
{
-  return getndelim2 (lineptr, linesize, (size_t)(-1), stream, '\n', 0, 0);
+  return getndelim2 (lineptr, linesize, 0, GETNDELIM_NO_LIMIT, '\n', 0,
+                     stream);
}

ssize_t
getdelim (char **lineptr, size_t *linesize, int delimiter, FILE *stream)
{
-  return getndelim2 (lineptr, linesize, (size_t)(-1), stream, delimiter, 0, 0);
+  return getndelim2 (lineptr, linesize, 0, GETNDELIM_NO_LIMIT, delimiter, 0,
+                     stream);
}
#endif
Index: lib/getline.h
===================================================================
RCS file: /cvsroot/gnulib/gnulib/lib/getline.h,v
retrieving revision 1.14
diff -u -r1.14 getline.h
--- lib/getline.h       24 Jul 2003 15:23:15 -0000      1.14
+++ lib/getline.h       24 Jul 2003 16:19:42 -0000
@@ -20,6 +20,10 @@
#ifndef GETLINE_H_
# define GETLINE_H_ 1

+#if HAVE_CONFIG_H
+# include <config.h>
+#endif
+
# include <stddef.h>
# include <stdio.h>

@@ -27,13 +31,17 @@
# include <sys/types.h>

/* glibc2 has these functions declared in <stdio.h>.  Avoid redeclarations.  */
-# if __GLIBC__ < 2
+# if !HAVE_WORKING_GETLINE

extern ssize_t getline (char **_lineptr, size_t *_linesize, FILE *_stream);

+# endif /* ! have getline */
+
+# if !HAVE_GETDELIM
+
extern ssize_t getdelim (char **_lineptr, size_t *_linesize, int _delimiter,
-                         FILE *_stream);
+                        FILE *_stream);

-# endif
+# endif /* ! have getdelim */

#endif /* not GETLINE_H_ */
Index: lib/getndelim2.c
===================================================================
RCS file: /cvsroot/gnulib/gnulib/lib/getndelim2.c,v
retrieving revision 1.2
diff -u -r1.2 getndelim2.c
--- lib/getndelim2.c    18 Jul 2003 16:58:06 -0000      1.2
+++ lib/getndelim2.c    24 Jul 2003 16:19:43 -0000
@@ -1,5 +1,5 @@
-/* getndelim2 - Read a line from a stream, stopping at one of 2 delimiters,
-   with bounded memory allocation.
+/* getndelim2 - Read n characters or less from a stream, stopping at one of up
+   to two specified delimiters.

   Copyright (C) 1993, 1996, 1997, 1998, 2000, 2003 Free Software
   Foundation, Inc.
@@ -39,23 +39,18 @@
#define MIN_CHUNK 64

ssize_t
-getndelim2 (char **lineptr, size_t *linesize, size_t nmax,
-           FILE *stream, int delim1, int delim2, size_t offset)
+getndelim2 (char **lineptr, size_t *linesize, size_t offset, size_t limit,
+            int delim1, int delim2, FILE *stream)
{
  size_t nbytes_avail;          /* Allocated but unused chars in *LINEPTR.  */
  char *read_pos;               /* Where we're reading into *LINEPTR. */

-  if (!lineptr || !linesize || !nmax || !stream)
+  if (!lineptr || !linesize || !stream)
    return -1;

  if (!*lineptr)
    {
-      size_t newlinesize = MIN_CHUNK;
-
-      if (newlinesize > nmax)
-       newlinesize = nmax;
-
-      *linesize = newlinesize;
+      *linesize = MIN_CHUNK;
      *lineptr = malloc (*linesize);
      if (!*lineptr)
        return -1;
@@ -67,36 +62,35 @@
  nbytes_avail = *linesize - offset;
  read_pos = *lineptr + offset;

-  if (nbytes_avail == 0 && *linesize >= nmax)
-    return -1;
-
  for (;;)
    {
      /* Here always *lineptr + *linesize == read_pos + nbytes_avail.  */
+      register int c;
+
+      if (limit == 0)
+       break;
+
+      c = getc (stream);

-      register int c = getc (stream);
+      if (limit != GETNDELIM_NO_LIMIT)
+       limit--;

      /* We always want at least one char left in the buffer, since we
         always (unless we get an error while reading the first char)
         NUL-terminate the line buffer.  */

-      if (nbytes_avail < 2 && *linesize < nmax)
+      if (nbytes_avail < 2)
        {
-         size_t newlinesize =
-           (*linesize > MIN_CHUNK ? 2 * *linesize : *linesize + MIN_CHUNK);
-
-         if (newlinesize > nmax)
-           newlinesize = nmax;
+         if (*linesize > MIN_CHUNK)
+           *linesize *= 2;
+         else
+           *linesize += MIN_CHUNK;

-         if (newlinesize > *linesize)
-           {
-             *linesize = newlinesize;
-             nbytes_avail = *linesize + *lineptr - read_pos;
-             *lineptr = realloc (*lineptr, *linesize);
-             if (!*lineptr)
-               return -1;
-             read_pos = *linesize - nbytes_avail + *lineptr;
-           }
+         nbytes_avail = *linesize + *lineptr - read_pos;
+         *lineptr = realloc (*lineptr, *linesize);
+         if (!*lineptr)
+           return -1;
+         read_pos = *linesize - nbytes_avail + *lineptr;
        }

      if (c == EOF || ferror (stream))
@@ -108,11 +102,8 @@
            break;
        }

-      if (nbytes_avail >= 2)
-       {
-         *read_pos++ = c;
-         nbytes_avail--;
-       }
+      *read_pos++ = c;
+      nbytes_avail--;

      if (c == delim1 || (delim2 && c == delim2))
        /* Return the line.  */
Index: lib/getndelim2.h
===================================================================
RCS file: /cvsroot/gnulib/gnulib/lib/getndelim2.h,v
retrieving revision 1.1
diff -u -r1.1 getndelim2.h
--- lib/getndelim2.h    18 Jul 2003 16:58:06 -0000      1.1
+++ lib/getndelim2.h    24 Jul 2003 16:19:43 -0000
@@ -1,5 +1,5 @@
-/* getndelim2 - Read a line from a stream, stopping at one of 2 delimiters,
-   with bounded memory allocation.
+/* getndelim2 - Read n characters or less from a stream, stopping at one of up
+   to two specified delimiters.

   Copyright (C) 2003 Free Software Foundation, Inc.

@@ -26,17 +26,17 @@
/* Get ssize_t.  */
#include <sys/types.h>

+#define GETNDELIM_NO_LIMIT (ssize_t)-1
+
/* Read up to (and including) a delimiter DELIM1 from STREAM into *LINEPTR
   + OFFSET (and NUL-terminate it).  If DELIM2 is non-zero, then read up
   and including the first occurrence of DELIM1 or DELIM2.  *LINEPTR is
   a pointer returned from malloc (or NULL), pointing to *LINESIZE bytes of
-   space.  It is realloc'd as necessary.  Reallocation is limited to
-   NMAX bytes; if the line is longer than that, the extra bytes are read but
-   thrown away.
+   space.  It is realloc'd as necessary.  Read no more than LIMIT bytes.
   Return the number of bytes read and stored at *LINEPTR + OFFSET (not
   including the NUL terminator), or -1 on error or EOF.  */
-extern ssize_t getndelim2 (char **lineptr, size_t *linesize, size_t nmax,
-                          FILE *stream, int delim1, int delim2,
-                          size_t offset);
+extern ssize_t getndelim2 (char **_lineptr, size_t *_linesize, size_t _offset,
+                           size_t _limit, int _delim1, int _delim2,
+                           FILE *_stream);

#endif /* GETNDELIM2_H */
Index: lib/getnline.c
===================================================================
RCS file: /cvsroot/gnulib/gnulib/lib/getnline.c,v
retrieving revision 1.2
diff -u -r1.2 getnline.c
--- lib/getnline.c      18 Jul 2003 16:58:06 -0000      1.2
+++ lib/getnline.c      24 Jul 2003 16:19:43 -0000
@@ -1,4 +1,4 @@
-/* getnline - Read a line from a stream, with bounded memory allocation.
+/* getnline - Read a line of n characters or less from a stream.

   Copyright (C) 2003 Free Software Foundation, Inc.

@@ -26,14 +26,14 @@
#include "getndelim2.h"

ssize_t
-getnline (char **lineptr, size_t *linesize, size_t nmax, FILE *stream)
+getnline (char **lineptr, size_t *linesize, size_t limit, FILE *stream)
{
-  return getndelim2 (lineptr, linesize, (size_t)(-1), stream, '\n', 0, 0);
+  return getndelim2 (lineptr, linesize, 0, limit, '\n', 0, stream);
}

ssize_t
-getndelim (char **lineptr, size_t *linesize, size_t nmax,
-          int delimiter, FILE *stream)
+getndelim (char **lineptr, size_t *linesize, size_t limit, int delimiter,
+           FILE *stream)
{
-  return getndelim2 (lineptr, linesize, (size_t)(-1), stream, delimiter, 0, 0);
+  return getndelim2 (lineptr, linesize, 0, limit, delimiter, 0, stream);
}
Index: lib/getnline.h
===================================================================
RCS file: /cvsroot/gnulib/gnulib/lib/getnline.h,v
retrieving revision 1.1
diff -u -r1.1 getnline.h
--- lib/getnline.h      17 Jul 2003 16:23:52 -0000      1.1
+++ lib/getnline.h      24 Jul 2003 16:19:43 -0000
@@ -1,4 +1,4 @@
-/* getnline - Read a line from a stream, with bounded memory allocation.
+/* getnline - Read a line of n characters or less from a stream.

   Copyright (C) 2003 Free Software Foundation, Inc.

@@ -27,23 +27,21 @@

/* Read a line, up to the next newline, from STREAM, and store it in *LINEPTR.
   *LINEPTR is a pointer returned from malloc (or NULL), pointing to *LINESIZE
-   bytes of space.  It is realloc'd as necessary.  Reallocation is limited to
-   NMAX bytes; if the line is longer than that, the extra bytes are read but
-   thrown away.
+   bytes of space.  It is realloc'd as necessary.  Read a maximum of LIMIT
+   bytes.
   Return the number of bytes read and stored at *LINEPTR (not including the
   NUL terminator), or -1 on error or EOF.  */
-extern ssize_t getnline (char **lineptr, size_t *linesize, size_t nmax,
-                        FILE *stream);
+extern ssize_t getnline (char **_lineptr, size_t *_linesize, size_t limit,
+                         FILE *_stream);

/* Read a line, up to the next occurrence of DELIMITER, from STREAM, and store
   it in *LINEPTR.
   *LINEPTR is a pointer returned from malloc (or NULL), pointing to *LINESIZE
-   bytes of space.  It is realloc'd as necessary.  Reallocation is limited to
-   NMAX bytes; if the line is longer than that, the extra bytes are read but
-   thrown away.
+   bytes of space.  It is realloc'd as necessary.  Read a maximum of LIMIT
+   bytes.
   Return the number of bytes read and stored at *LINEPTR (not including the
   NUL terminator), or -1 on error or EOF.  */
-extern ssize_t getndelim (char **lineptr, size_t *linesize, size_t nmax,
-                         int delimiter, FILE *stream);
+extern ssize_t getndelim (char **_lineptr, size_t *_linesize, size_t limit,
+                          int _delimiter, FILE *_stream);

#endif /* GETNLINE_H */



Derek

--
               *8^)

Email: address@hidden

Get CVS support at <http://ximbiot.com>!
--
The Christmas Pageant does not stink.
The Christmas Pageant does not stink.
The Christmas Pageant does not stink...

         - Bart Simpson on chalkboard, _The Simpsons_