[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Bug-gnulib] new module: getsubopt

From: Paul Eggert
Subject: Re: [Bug-gnulib] new module: getsubopt
Date: Sun, 01 Aug 2004 16:31:52 -0700
User-agent: Gnus/5.1006 (Gnus v5.10.6) Emacs/21.3 (gnu/linux)

Simon Josefsson <address@hidden> writes:

> * I didn't really understand the "restrict" stuff, so perhaps the
>   prototype in getsubopt.c should be changed.

The "restrict" keywords tell the caller that the arguments to
getsubopt cannot point at overlapping storage.  However, the POSIX
specification doesn't have "restrict", so the use of "restrict" here
violates POSIX.  This appears to be a bug in glibc; you might as well
fix it while you're at it, by removing "restrict".

>   Perhaps it should be changed anyway, it is K&R now.

Yes, please use prototypes, not K&R.  We assume C89 or better now.

> * The C file is glibc stdlib/getsubopt.c, but slightly modified.

Please arrange things so that the exact same file can be used in
gnulib versus glibc, except for the licensing comment.  That way, we
can list it in gnulib/config/srclist.txt, and it can be kept in sync
with glibc.

To that end, please replace this patch:

  +/* Get strchrnul.  */
  +#include <strchrnul.h>
  -  endp = __strchrnul (*optionp, ',');
  +  endp = strchrnul (*optionp, ',');

with something like this instead.  (The "Get" comment is redundant so
we can remove it.)

  #if !_LIBC
  # include <strchrnul.h>
  # define __strchrnul strchrnul

Also, I noticed a violation of the C Standard in the implementation.
This call:

    if (memcmp (*optionp, tokens[cnt], vstart - *optionp) == 0

has undefined behavior, since vstart - *options might be much longer
than strlen (tokens[cnt]).  This isn't a bug in glibc, since its
memcmp always goes left-to-right, but it is a bug in general.  Please
fix this bug by replacing memcmp with strncmp.

> * MODULES.html.sh?  It mention getsubopt, but I dunno where to add
>   getsubopt.  IIUC, getsubopt is X/Open, not POSIX.

These days it's specified by POSIX, as an XSI extension, here:
So I'd add it to the section "Support for systems lacking POSIX:2001".

reply via email to

[Prev in Thread] Current Thread [Next in Thread]