Re: [Bug-gnulib] xreadlink.c patch

[Mark D. Baushke]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Paul Eggert <address@hidden> writes:

> "Mark D. Baushke" <address@hidden> writes:
> 
> >   1) bugfix. readlink() on AIX 4.3 returns a
> >      negative link_length and sets errno == ERANGE
> >      when the length of the link is greater than
> >      buf_size.
> 
> Shouldn't this incompatibility be fixed in readlink.c rather than
> xreadlink?  I would expect other users of the readlink module to be
> affected by it.  Perhaps Bruno can comment, since he did readlink.c.

The existing readlink.c is a stub for systems that do not have one. I
would expect there to need to be a rpl_readlink version if it were going
to adapt to the behavior of dealing with incompatible behaviors.

> >   2) enhancement. The size passed to xreadlink
> >      could be the maximum value and adding one
> >      could wrap it to zero which would be a bad
> >      idea.
> >
> >   3) enhancement. Allow for at least one attempt
> >      at the maximum allowed buffer size if
> >      doubling the current buf_size pushes over the
> >      limit.
> 
> These are both good suggestions, but there's a problem with that
> patch: it assumes SSIZE_MAX < SIZE_MAX, but POSIX does not require
> this.  I installed the following patch instead.

If POSIX does not require it, does that imply that you may be truncating
the result when you do the link_length = r  assignment? That is, will
SSIZE_MAX > SIZE_MAX ever be true?

> 2004-11-02  Paul Eggert  <address@hidden>
> 
>       * xreadlink.c (MAXSIZE): New macro.
>       (xreadlink): Use it instead of SSIZE_MAX.  Ensure initial buffer
>       size does not exceed MAXSIZE.  Avoid cast.
>       As suggested by Mark D. Baushke in
>       <http://lists.gnu.org/archive/html/bug-gnulib/2004-11/msg00009.html>,
>       if readlink fails with buffer size just under MAXSIZE, try again
>       with MAXSIZE.

Thanks for the patch.

Query: Should xreadlink.c do

#include "xsize.h"

instead of open coding the

#ifndef SIZE_MAX
# define SIZE_MAX ((size_t) -1)
#endif
#ifndef SSIZE_MAX
# define SSIZE_MAX ((ssize_t) (SIZE_MAX / 2))
#endif

lines?

As another optimization, I am curious to know if it makes more sense to
use xrealloc() which may have allocated enough space for more than one
attempt for the readlink call?

        -- Mark

/* xreadlink.c -- readlink wrapper to return the link name in malloc'd storage

   Copyright (C) 2001, 2003, 2004 Free Software Foundation, Inc.

   This program is free software; you can redistribute it and/or modify
   it under the terms of the GNU General Public License as published by
   the Free Software Foundation; either version 2, or (at your option)
   any later version.

   This program is distributed in the hope that it will be useful,
   but WITHOUT ANY WARRANTY; without even the implied warranty of
   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
   GNU General Public License for more details.

   You should have received a copy of the GNU General Public License
   along with this program; see the file COPYING.
   If not, write to the Free Software Foundation,
   59 Temple Place - Suite 330, Boston, MA 02111-1307, USA.  */

/* Written by Jim Meyering <address@hidden>  */

#if HAVE_CONFIG_H
# include <config.h>
#endif

#include "xreadlink.h"

#include <stdio.h>
#include <errno.h>
#include <limits.h>
#include <sys/types.h>
#include <stdlib.h>
#if HAVE_UNISTD_H
# include <unistd.h>
#endif

#include "xsize.h"

#define MAXSIZE (SIZE_MAX < SSIZE_MAX ? SIZE_MAX : SSIZE_MAX)

#include "xalloc.h"

/* Call readlink to get the symbolic link value of FILENAME.
   SIZE is a hint as to how long the link is expected to be;
   typically it is taken from st_size.  It need not be correct.
   Return a pointer to that NUL-terminated string in malloc'd storage.
   If readlink fails, return NULL (caller may use errno to diagnose).
   If malloc fails, or if the link value is longer than SSIZE_MAX :-),
   give a diagnostic and exit.  */

char *
xreadlink (char const *filename, size_t size)
{
  /* The initial buffer size for the link value.  A power of 2
     detects arithmetic overflow earlier, but is not required.  */
  size_t buf_size = size < MAXSIZE ? size + 1 : MAXSIZE;
  char *buffer = NULL;

  while (1)
    {
      buffer = xrealloc (buffer, buf_size);
      ssize_t r = readlink (filename, buffer, buf_size);
      size_t link_length = r;

      if (r < 0)
        {
          int saved_errno = errno;
          free (buffer);
          errno = saved_errno;
          return NULL;
        }

      if (link_length < buf_size)
        {
          buffer[link_length] = 0;
          return buffer;
        }

      if (buf_size <= MAXSIZE / 2)
        buf_size *= 2;
      else if (buf_size < MAXSIZE)
        buf_size = MAXSIZE;
      else
        xalloc_die ();
    }
}

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.2.3 (FreeBSD)

iD8DBQFBiBTQ3x41pRYZE/gRAgiMAKCm3DDDpdAaM7MpP2ErL94F1oEyxACgqJYf
0vyrIVfx6rs6cCSsol7d028=
=R6H9
-----END PGP SIGNATURE-----