public inbox for libc-alpha@sourceware.org
 help / color / mirror / Atom feed
From: Carlos O'Donell <carlos@redhat.com>
To: Xi Ruoyao <xry111@xry111.site>, libc-alpha@sourceware.org
Cc: Adhemerval Zanella Netto <adhemerval.zanella@linaro.org>,
	Alex Colomar <alx.manpages@gmail.com>
Subject: Re: [PATCH v2] libio: Add nonnull attribute for most FILE * arguments in stdio.h
Date: Mon, 29 May 2023 09:58:37 -0400	[thread overview]
Message-ID: <092bae7e-6e7b-d832-39ec-debb0ee6cfb8@redhat.com> (raw)
In-Reply-To: <20230519051940.17458-1-xry111@xry111.site>

On 5/19/23 01:19, Xi Ruoyao wrote:
> During the review of a GCC analyzer test case, we found most stdio
> functions accepting a FILE * argument expect it to be nonnull and just
> segfault when the argument is NULL.  Add nonnull attribute for them.
> 
> fflush and fflush_unlocked are well defined when __stream is NULL so
> they are not touched.
> 
> For fputs, fgets, fread, fwrite, fprintf, vfprintf, and their unlocked
> version, if __stream is empty but there is nothing to read or write,
> they don't segfault and I'm not sure if the standard allows such a use
> so I left them out.

May you please provided Signed-off-by for DCO or explain your FSF copyright assignment?

> ---
>  libio/stdio.h | 118 ++++++++++++++++++++++++++------------------------
>  1 file changed, 62 insertions(+), 56 deletions(-)
> 
> diff --git a/libio/stdio.h b/libio/stdio.h
> index 4cf9f1c012..49ba361931 100644
> --- a/libio/stdio.h
> +++ b/libio/stdio.h
> @@ -278,7 +278,7 @@ extern FILE *__REDIRECT (fopen, (const char *__restrict __filename,
>  extern FILE *__REDIRECT (freopen, (const char *__restrict __filename,
>  				   const char *__restrict __modes,
>  				   FILE *__restrict __stream), freopen64)
> -  __wur;
> +  __wur __nonnull ((3));
>  # else
>  #  define fopen fopen64
>  #  define freopen freopen64
> @@ -330,21 +330,22 @@ extern __FILE *open_wmemstream (wchar_t **__bufloc, size_t *__sizeloc) __THROW
>  
>  /* If BUF is NULL, make STREAM unbuffered.
>     Else make it use buffer BUF, of size BUFSIZ.  */
> -extern void setbuf (FILE *__restrict __stream, char *__restrict __buf) __THROW;
> +extern void setbuf (FILE *__restrict __stream, char *__restrict __buf) __THROW
> +  __nonnull ((1));
>  /* Make STREAM use buffering mode MODE.
>     If BUF is not NULL, use N bytes of it for buffering;
>     else allocate an internal buffer N bytes long.  */
>  extern int setvbuf (FILE *__restrict __stream, char *__restrict __buf,
> -		    int __modes, size_t __n) __THROW;
> +		    int __modes, size_t __n) __THROW __nonnull ((1));
>  
>  #ifdef	__USE_MISC
>  /* If BUF is NULL, make STREAM unbuffered.
>     Else make it use SIZE bytes of BUF for buffering.  */
>  extern void setbuffer (FILE *__restrict __stream, char *__restrict __buf,
> -		       size_t __size) __THROW;
> +		       size_t __size) __THROW __nonnull ((1));
>  
>  /* Make STREAM line-buffered.  */
> -extern void setlinebuf (FILE *__stream) __THROW;
> +extern void setlinebuf (FILE *__stream) __THROW __nonnull ((1));
>  #endif
>  
>  
> @@ -418,7 +419,7 @@ extern int dprintf (int __fd, const char *__restrict __fmt, ...)
>     This function is a possible cancellation point and therefore not
>     marked with __THROW.  */
>  extern int fscanf (FILE *__restrict __stream,
> -		   const char *__restrict __format, ...) __wur;
> +		   const char *__restrict __format, ...) __wur __nonnull ((1));
>  /* Read formatted input from stdin.
>  
>     This function is a possible cancellation point and therefore not
> @@ -439,7 +440,7 @@ extern int sscanf (const char *__restrict __s,
>  #  ifdef __REDIRECT
>  extern int __REDIRECT (fscanf, (FILE *__restrict __stream,
>  				const char *__restrict __format, ...),
> -		       __isoc23_fscanf) __wur;
> +		       __isoc23_fscanf) __wur __nonnull ((1));
>  extern int __REDIRECT (scanf, (const char *__restrict __format, ...),
>  		       __isoc23_scanf) __wur;
>  extern int __REDIRECT_NTH (sscanf, (const char *__restrict __s,
> @@ -447,7 +448,7 @@ extern int __REDIRECT_NTH (sscanf, (const char *__restrict __s,
>  			   __isoc23_sscanf);
>  #  else
>  extern int __isoc23_fscanf (FILE *__restrict __stream,
> -			    const char *__restrict __format, ...) __wur;
> +			    const char *__restrict __format, ...) __wur __nonnull ((1));
>  extern int __isoc23_scanf (const char *__restrict __format, ...) __wur;
>  extern int __isoc23_sscanf (const char *__restrict __s,
>  			    const char *__restrict __format, ...) __THROW;
> @@ -459,7 +460,7 @@ extern int __isoc23_sscanf (const char *__restrict __s,
>  #  ifdef __REDIRECT
>  extern int __REDIRECT (fscanf, (FILE *__restrict __stream,
>  				const char *__restrict __format, ...),
> -		       __isoc99_fscanf) __wur;
> +		       __isoc99_fscanf) __wur __nonnull ((1));
>  extern int __REDIRECT (scanf, (const char *__restrict __format, ...),
>  		       __isoc99_scanf) __wur;
>  extern int __REDIRECT_NTH (sscanf, (const char *__restrict __s,
> @@ -467,7 +468,7 @@ extern int __REDIRECT_NTH (sscanf, (const char *__restrict __s,
>  			   __isoc99_sscanf);
>  #  else
>  extern int __isoc99_fscanf (FILE *__restrict __stream,
> -			    const char *__restrict __format, ...) __wur;
> +			    const char *__restrict __format, ...) __wur __nonnull ((1));
>  extern int __isoc99_scanf (const char *__restrict __format, ...) __wur;
>  extern int __isoc99_sscanf (const char *__restrict __s,
>  			    const char *__restrict __format, ...) __THROW;
> @@ -485,7 +486,7 @@ extern int __isoc99_sscanf (const char *__restrict __s,
>     marked with __THROW.  */
>  extern int vfscanf (FILE *__restrict __s, const char *__restrict __format,
>  		    __gnuc_va_list __arg)
> -     __attribute__ ((__format__ (__scanf__, 2, 0))) __wur;
> +     __attribute__ ((__format__ (__scanf__, 2, 0))) __wur __nonnull ((1));
>  
>  /* Read formatted input from stdin into argument list ARG.
>  
> @@ -508,7 +509,7 @@ extern int __REDIRECT (vfscanf,
>  		       (FILE *__restrict __s,
>  			const char *__restrict __format, __gnuc_va_list __arg),
>  		       __isoc23_vfscanf)
> -     __attribute__ ((__format__ (__scanf__, 2, 0))) __wur;
> +     __attribute__ ((__format__ (__scanf__, 2, 0))) __wur __nonnull ((1));
>  extern int __REDIRECT (vscanf, (const char *__restrict __format,
>  				__gnuc_va_list __arg), __isoc23_vscanf)
>       __attribute__ ((__format__ (__scanf__, 1, 0))) __wur;
> @@ -520,7 +521,7 @@ extern int __REDIRECT_NTH (vsscanf,
>  #   elif !defined __REDIRECT
>  extern int __isoc23_vfscanf (FILE *__restrict __s,
>  			     const char *__restrict __format,
> -			     __gnuc_va_list __arg) __wur;
> +			     __gnuc_va_list __arg) __wur __nonnull ((1));
>  extern int __isoc23_vscanf (const char *__restrict __format,
>  			    __gnuc_va_list __arg) __wur;
>  extern int __isoc23_vsscanf (const char *__restrict __s,
> @@ -537,7 +538,7 @@ extern int __REDIRECT (vfscanf,
>  		       (FILE *__restrict __s,
>  			const char *__restrict __format, __gnuc_va_list __arg),
>  		       __isoc99_vfscanf)
> -     __attribute__ ((__format__ (__scanf__, 2, 0))) __wur;
> +     __attribute__ ((__format__ (__scanf__, 2, 0))) __wur __nonnull ((1));
>  extern int __REDIRECT (vscanf, (const char *__restrict __format,
>  				__gnuc_va_list __arg), __isoc99_vscanf)
>       __attribute__ ((__format__ (__scanf__, 1, 0))) __wur;
> @@ -549,7 +550,7 @@ extern int __REDIRECT_NTH (vsscanf,
>  #   elif !defined __REDIRECT
>  extern int __isoc99_vfscanf (FILE *__restrict __s,
>  			     const char *__restrict __format,
> -			     __gnuc_va_list __arg) __wur;
> +			     __gnuc_va_list __arg) __wur __nonnull ((1));
>  extern int __isoc99_vscanf (const char *__restrict __format,
>  			    __gnuc_va_list __arg) __wur;
>  extern int __isoc99_vsscanf (const char *__restrict __s,
> @@ -568,8 +569,8 @@ extern int __isoc99_vsscanf (const char *__restrict __s,
>  
>     These functions are possible cancellation points and therefore not
>     marked with __THROW.  */
> -extern int fgetc (FILE *__stream);
> -extern int getc (FILE *__stream);
> +extern int fgetc (FILE *__stream) __nonnull ((1));
> +extern int getc (FILE *__stream) __nonnull ((1));
>  
>  /* Read a character from stdin.
>  
> @@ -582,7 +583,7 @@ extern int getchar (void);
>  
>     These functions are possible cancellation points and therefore not
>     marked with __THROW.  */
> -extern int getc_unlocked (FILE *__stream);
> +extern int getc_unlocked (FILE *__stream) __nonnull ((1));
>  extern int getchar_unlocked (void);
>  #endif /* Use POSIX.  */
>  
> @@ -593,7 +594,7 @@ extern int getchar_unlocked (void);
>     cancellation point.  But due to similarity with an POSIX interface
>     or due to the implementation it is a cancellation point and
>     therefore not marked with __THROW.  */
> -extern int fgetc_unlocked (FILE *__stream);
> +extern int fgetc_unlocked (FILE *__stream) __nonnull ((1));
>  #endif /* Use MISC.  */
>  
>  
> @@ -604,8 +605,8 @@ extern int fgetc_unlocked (FILE *__stream);
>  
>     These functions is a possible cancellation point and therefore not
>     marked with __THROW.  */
> -extern int fputc (int __c, FILE *__stream);
> -extern int putc (int __c, FILE *__stream);
> +extern int fputc (int __c, FILE *__stream) __nonnull ((2));
> +extern int putc (int __c, FILE *__stream) __nonnull ((2));
>  
>  /* Write a character to stdout.
>  
> @@ -620,7 +621,7 @@ extern int putchar (int __c);
>     cancellation point.  But due to similarity with an POSIX interface
>     or due to the implementation it is a cancellation point and
>     therefore not marked with __THROW.  */
> -extern int fputc_unlocked (int __c, FILE *__stream);
> +extern int fputc_unlocked (int __c, FILE *__stream) __nonnull ((2));
>  #endif /* Use MISC.  */
>  
>  #ifdef __USE_POSIX199506
> @@ -628,7 +629,7 @@ extern int fputc_unlocked (int __c, FILE *__stream);
>  
>     These functions are possible cancellation points and therefore not
>     marked with __THROW.  */
> -extern int putc_unlocked (int __c, FILE *__stream);
> +extern int putc_unlocked (int __c, FILE *__stream) __nonnull ((2));
>  extern int putchar_unlocked (int __c);
>  #endif /* Use POSIX.  */
>  
> @@ -636,10 +637,10 @@ extern int putchar_unlocked (int __c);
>  #if defined __USE_MISC \
>      || (defined __USE_XOPEN && !defined __USE_XOPEN2K)
>  /* Get a word (int) from STREAM.  */
> -extern int getw (FILE *__stream);
> +extern int getw (FILE *__stream) __nonnull ((1));
>  
>  /* Write a word (int) to STREAM.  */
> -extern int putw (int __w, FILE *__stream);
> +extern int putw (int __w, FILE *__stream) __nonnull ((2));
>  #endif
>  
>  
> @@ -689,10 +690,10 @@ extern char *fgets_unlocked (char *__restrict __s, int __n,
>     therefore not marked with __THROW.  */
>  extern __ssize_t __getdelim (char **__restrict __lineptr,
>                               size_t *__restrict __n, int __delimiter,
> -                             FILE *__restrict __stream) __wur;
> +                             FILE *__restrict __stream) __wur __nonnull ((4));
>  extern __ssize_t getdelim (char **__restrict __lineptr,
>                             size_t *__restrict __n, int __delimiter,
> -                           FILE *__restrict __stream) __wur;
> +                           FILE *__restrict __stream) __wur __nonnull ((4));
>  
>  /* Like `getdelim', but reads up to a newline.
>  
> @@ -702,7 +703,7 @@ extern __ssize_t getdelim (char **__restrict __lineptr,
>     therefore not marked with __THROW.  */
>  extern __ssize_t getline (char **__restrict __lineptr,
>                            size_t *__restrict __n,
> -                          FILE *__restrict __stream) __wur;
> +                          FILE *__restrict __stream) __wur __nonnull ((3));
>  #endif
>  
>  
> @@ -723,7 +724,7 @@ extern int puts (const char *__s);
>  
>     This function is a possible cancellation point and therefore not
>     marked with __THROW.  */
> -extern int ungetc (int __c, FILE *__stream);
> +extern int ungetc (int __c, FILE *__stream) __nonnull ((2));
>  
>  
>  /* Read chunks of generic data from STREAM.
> @@ -768,17 +769,17 @@ extern size_t fwrite_unlocked (const void *__restrict __ptr, size_t __size,
>  
>     This function is a possible cancellation point and therefore not
>     marked with __THROW.  */
> -extern int fseek (FILE *__stream, long int __off, int __whence);
> +extern int fseek (FILE *__stream, long int __off, int __whence) __nonnull ((1));
>  /* Return the current position of STREAM.
>  
>     This function is a possible cancellation point and therefore not
>     marked with __THROW.  */
> -extern long int ftell (FILE *__stream) __wur;
> +extern long int ftell (FILE *__stream) __wur __nonnull ((1));
>  /* Rewind to the beginning of STREAM.
>  
>     This function is a possible cancellation point and therefore not
>     marked with __THROW.  */
> -extern void rewind (FILE *__stream);
> +extern void rewind (FILE *__stream) __nonnull ((1));
>  
>  /* The Single Unix Specification, Version 2, specifies an alternative,
>     more adequate interface for the two functions above which deal with
> @@ -791,18 +792,19 @@ extern void rewind (FILE *__stream);
>  
>     This function is a possible cancellation point and therefore not
>     marked with __THROW.  */
> -extern int fseeko (FILE *__stream, __off_t __off, int __whence);
> +extern int fseeko (FILE *__stream, __off_t __off, int __whence) __nonnull ((1));
>  /* Return the current position of STREAM.
>  
>     This function is a possible cancellation point and therefore not
>     marked with __THROW.  */
> -extern __off_t ftello (FILE *__stream) __wur;
> +extern __off_t ftello (FILE *__stream) __wur __nonnull ((1));
>  # else
>  #  ifdef __REDIRECT
>  extern int __REDIRECT (fseeko,
>  		       (FILE *__stream, __off64_t __off, int __whence),
> -		       fseeko64);
> -extern __off64_t __REDIRECT (ftello, (FILE *__stream), ftello64);
> +		       fseeko64) __nonnull ((1));
> +extern __off64_t __REDIRECT (ftello, (FILE *__stream), ftello64)
> +  __nonnull ((1));
>  #  else
>  #   define fseeko fseeko64
>  #   define ftello ftello64
> @@ -815,18 +817,20 @@ extern __off64_t __REDIRECT (ftello, (FILE *__stream), ftello64);
>  
>     This function is a possible cancellation point and therefore not
>     marked with __THROW.  */
> -extern int fgetpos (FILE *__restrict __stream, fpos_t *__restrict __pos);
> +extern int fgetpos (FILE *__restrict __stream, fpos_t *__restrict __pos)
> +  __nonnull ((1));
>  /* Set STREAM's position.
>  
>     This function is a possible cancellation point and therefore not
>     marked with __THROW.  */
> -extern int fsetpos (FILE *__stream, const fpos_t *__pos);
> +extern int fsetpos (FILE *__stream, const fpos_t *__pos) __nonnull ((1));
>  #else
>  # ifdef __REDIRECT
>  extern int __REDIRECT (fgetpos, (FILE *__restrict __stream,
> -				 fpos_t *__restrict __pos), fgetpos64);
> +				 fpos_t *__restrict __pos), fgetpos64) __nonnull ((1));
>  extern int __REDIRECT (fsetpos,
> -		       (FILE *__stream, const fpos_t *__pos), fsetpos64);
> +		       (FILE *__stream, const fpos_t *__pos), fsetpos64)
> +  __nonnull ((1));
>  # else
>  #  define fgetpos fgetpos64
>  #  define fsetpos fsetpos64
> @@ -834,24 +838,26 @@ extern int __REDIRECT (fsetpos,
>  #endif
>  
>  #ifdef __USE_LARGEFILE64
> -extern int fseeko64 (FILE *__stream, __off64_t __off, int __whence);
> -extern __off64_t ftello64 (FILE *__stream) __wur;
> -extern int fgetpos64 (FILE *__restrict __stream, fpos64_t *__restrict __pos);
> -extern int fsetpos64 (FILE *__stream, const fpos64_t *__pos);
> +extern int fseeko64 (FILE *__stream, __off64_t __off, int __whence)
> +  __nonnull ((1));
> +extern __off64_t ftello64 (FILE *__stream) __wur __nonnull ((1));
> +extern int fgetpos64 (FILE *__restrict __stream, fpos64_t *__restrict __pos)
> +  __nonnull ((1));
> +extern int fsetpos64 (FILE *__stream, const fpos64_t *__pos) __nonnull ((1));
>  #endif
>  
>  /* Clear the error and EOF indicators for STREAM.  */
> -extern void clearerr (FILE *__stream) __THROW;
> +extern void clearerr (FILE *__stream) __THROW __nonnull ((1));
>  /* Return the EOF indicator for STREAM.  */
> -extern int feof (FILE *__stream) __THROW __wur;
> +extern int feof (FILE *__stream) __THROW __wur __nonnull ((1));
>  /* Return the error indicator for STREAM.  */
> -extern int ferror (FILE *__stream) __THROW __wur;
> +extern int ferror (FILE *__stream) __THROW __wur __nonnull ((1));
>  
>  #ifdef __USE_MISC
>  /* Faster versions when locking is not required.  */
> -extern void clearerr_unlocked (FILE *__stream) __THROW;
> -extern int feof_unlocked (FILE *__stream) __THROW __wur;
> -extern int ferror_unlocked (FILE *__stream) __THROW __wur;
> +extern void clearerr_unlocked (FILE *__stream) __THROW __nonnull ((1));
> +extern int feof_unlocked (FILE *__stream) __THROW __wur __nonnull ((1));
> +extern int ferror_unlocked (FILE *__stream) __THROW __wur __nonnull ((1));
>  #endif
>  
>  
> @@ -864,12 +870,12 @@ extern void perror (const char *__s) __COLD;
>  
>  #ifdef	__USE_POSIX
>  /* Return the system file descriptor for STREAM.  */
> -extern int fileno (FILE *__stream) __THROW __wur;
> +extern int fileno (FILE *__stream) __THROW __wur __nonnull ((1));
>  #endif /* Use POSIX.  */
>  
>  #ifdef __USE_MISC
>  /* Faster version when locking is not required.  */
> -extern int fileno_unlocked (FILE *__stream) __THROW __wur;
> +extern int fileno_unlocked (FILE *__stream) __THROW __wur __nonnull ((1));
>  #endif
>  
>  
> @@ -878,7 +884,7 @@ extern int fileno_unlocked (FILE *__stream) __THROW __wur;
>  
>     This function is a possible cancellation point and therefore not
>     marked with __THROW.  */
> -extern int pclose (FILE *__stream);
> +extern int pclose (FILE *__stream) __nonnull ((1));
>  
>  /* Create a new stream connected to a pipe running the given command.
>  
> @@ -922,14 +928,14 @@ extern int obstack_vprintf (struct obstack *__restrict __obstack,
>  /* These are defined in POSIX.1:1996.  */
>  
>  /* Acquire ownership of STREAM.  */
> -extern void flockfile (FILE *__stream) __THROW;
> +extern void flockfile (FILE *__stream) __THROW __nonnull ((1));
>  
>  /* Try to acquire ownership of STREAM but do not block if it is not
>     possible.  */
> -extern int ftrylockfile (FILE *__stream) __THROW __wur;
> +extern int ftrylockfile (FILE *__stream) __THROW __wur __nonnull ((1));
>  
>  /* Relinquish the ownership granted for STREAM.  */
> -extern void funlockfile (FILE *__stream) __THROW;
> +extern void funlockfile (FILE *__stream) __THROW __nonnull ((1));
>  #endif /* POSIX */
>  
>  #if defined __USE_XOPEN && !defined __USE_XOPEN2K && !defined __USE_GNU

-- 
Cheers,
Carlos.


  reply	other threads:[~2023-05-29 13:58 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-05-19  5:19 Xi Ruoyao
2023-05-29 13:58 ` Carlos O'Donell [this message]
2023-05-29 14:15   ` Xi Ruoyao
2023-05-29 18:49     ` Carlos O'Donell
2023-06-12 10:27 ` ping: " Xi Ruoyao
2023-06-19  9:13   ` ping^2: " Xi Ruoyao

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=092bae7e-6e7b-d832-39ec-debb0ee6cfb8@redhat.com \
    --to=carlos@redhat.com \
    --cc=adhemerval.zanella@linaro.org \
    --cc=alx.manpages@gmail.com \
    --cc=libc-alpha@sourceware.org \
    --cc=xry111@xry111.site \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).