Re: [PATCH] libio: Add nonnull attribute for most FILE * arguments in stdio.h

[Alejandro Colomar <alx.manpages@gmail.com>]

[-- Attachment #1.1: Type: text/plain, Size: 3605 bytes --]

On 5/18/23 20:06, Alex Colomar wrote:
> Hi Xi!
> 
> On 5/18/23 19:25, Xi Ruoyao via Libc-alpha 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.
>>
>> setbuf is well defined when __stream is NULL so it's 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.
>> ---
>>   libio/stdio.h | 119 ++++++++++++++++++++++++++------------------------
>>   1 file changed, 62 insertions(+), 57 deletions(-)
>>
>> diff --git a/libio/stdio.h b/libio/stdio.h
>> index 4cf9f1c012..ae3d7295d4 100644
>> --- a/libio/stdio.h
>> +++ b/libio/stdio.h
>> @@ -232,7 +232,7 @@ extern char *tempnam (const char *__dir, const 
>> char *__pfx)
>>      This function is a possible cancellation point and therefore not
>>      marked with __THROW.  */
>> -extern int fflush (FILE *__stream);
>> +extern int fflush (FILE *__stream) __nonnull ((1));
> 
> flush(NULL) is well defined.  It flushes all streams that can be
> flushed.  This reminds me that I should document that in the SYNOPSIS
> section of the manual page; an oversight on my side.

I just pushed this:

<https://git.kernel.org/pub/scm/docs/man-pages/man-pages.git/commit/?id=cd03c9b8d1f4d43ab7b010d02287dec1805ada34>

commit cd03c9b8d1f4d43ab7b010d02287dec1805ada34 (HEAD -> master, 
korg/master, alx/main, alx/HEAD, main)
Author: Alejandro Colomar <alx@kernel.org>
Date:   Thu May 18 20:53:14 2023 +0200

     fflush.3, unlocked_stdio.3: SYNOPSIS: The streams can be null

     Cc: Xi Ruoyao <xry111@xry111.site>
     Signed-off-by: Alejandro Colomar <alx@kernel.org>

diff --git a/man3/fflush.3 b/man3/fflush.3
index 327786cef..2098bba08 100644
--- a/man3/fflush.3
+++ b/man3/fflush.3
@@ -25,7 +25,7 @@ Standard C library
  .nf
  .B #include <stdio.h>
  .PP
-.BI "int fflush(FILE *" stream );
+.BI "int fflush(FILE *_Nullable " stream );
  .fi
  .SH DESCRIPTION
  For output streams,
diff --git a/man3/unlocked_stdio.3 b/man3/unlocked_stdio.3
index faab29f06..feed708db 100644
--- a/man3/unlocked_stdio.3
+++ b/man3/unlocked_stdio.3
@@ -23,7 +23,7 @@ Standard C library
  .BI "int feof_unlocked(FILE *" stream );
  .BI "int ferror_unlocked(FILE *" stream );
  .BI "int fileno_unlocked(FILE *" stream );
-.BI "int fflush_unlocked(FILE *" stream );
+.BI "int fflush_unlocked(FILE *_Nullable " stream );
  .PP
  .BI "int fgetc_unlocked(FILE *" stream );
  .BI "int fputc_unlocked(int " c ", FILE *" stream );

> 
>>   #ifdef __USE_MISC
>>   /* Faster versions when locking is not required.
>> @@ -241,7 +241,7 @@ extern int fflush (FILE *__stream);
>>      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 fflush_unlocked (FILE *__stream);
>> +extern int fflush_unlocked (FILE *__stream) __nonnull ((1));
> 
> Without checking, I'll guess that fflush_unlocked(NULL) is also well
> defined.
> 
> I didn't see any other similar cases, but I may have missed some; I
> didn't revise them all thoroughly; please check.
> 

-- 
<http://www.alejandro-colomar.es/>
GPG key fingerprint: A9348594CE31283A826FBDD8D57633D441E25BB5


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]