public inbox for libc-alpha@sourceware.org
 help / color / mirror / Atom feed
* [PATCH] libio: Add __nonnull for FILE * arguments of fclose and freopen
@ 2023-04-21  6:21 Xi Ruoyao
  2023-04-21 13:00 ` Adhemerval Zanella Netto
  2023-05-16  2:50 ` Ping: " Xi Ruoyao
  0 siblings, 2 replies; 6+ messages in thread
From: Xi Ruoyao @ 2023-04-21  6:21 UTC (permalink / raw)
  To: libc-alpha; +Cc: Adhemerval Zanella Netto, Xi Ruoyao

Calling fclose or freopen with a null FILE * is undefined behavior, and
doing so in practice will cause a SIGSEGV.  So it seems suitable for
__nonnull.

This will help the compiler to warn for some buggy code, like
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=109570.
---
 libio/stdio.h | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/libio/stdio.h b/libio/stdio.h
index 45ddafdf20..bfb8415d3b 100644
--- a/libio/stdio.h
+++ b/libio/stdio.h
@@ -180,7 +180,7 @@ extern int renameat2 (int __oldfd, const char *__old, int __newfd,
 
    This function is a possible cancellation point and therefore not
    marked with __THROW.  */
-extern int fclose (FILE *__stream);
+extern int fclose (FILE *__stream) __nonnull ((1));
 
 #undef __attr_dealloc_fclose
 #define __attr_dealloc_fclose __attr_dealloc (fclose, 1)
@@ -269,7 +269,7 @@ extern FILE *fopen (const char *__restrict __filename,
    marked with __THROW.  */
 extern FILE *freopen (const char *__restrict __filename,
 		      const char *__restrict __modes,
-		      FILE *__restrict __stream) __wur;
+		      FILE *__restrict __stream) __wur __nonnull ((3));
 #else
 # ifdef __REDIRECT
 extern FILE *__REDIRECT (fopen, (const char *__restrict __filename,
@@ -290,7 +290,7 @@ extern FILE *fopen64 (const char *__restrict __filename,
   __attribute_malloc__ __attr_dealloc_fclose __wur;
 extern FILE *freopen64 (const char *__restrict __filename,
 			const char *__restrict __modes,
-			FILE *__restrict __stream) __wur;
+			FILE *__restrict __stream) __wur __nonnull ((3));
 #endif
 
 #ifdef	__USE_POSIX
-- 
2.40.0


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] libio: Add __nonnull for FILE * arguments of fclose and freopen
  2023-04-21  6:21 [PATCH] libio: Add __nonnull for FILE * arguments of fclose and freopen Xi Ruoyao
@ 2023-04-21 13:00 ` Adhemerval Zanella Netto
  2023-04-22  7:32   ` Xi Ruoyao
  2023-05-16  2:50 ` Ping: " Xi Ruoyao
  1 sibling, 1 reply; 6+ messages in thread
From: Adhemerval Zanella Netto @ 2023-04-21 13:00 UTC (permalink / raw)
  To: Xi Ruoyao, libc-alpha



On 21/04/23 03:21, Xi Ruoyao wrote:
> Calling fclose or freopen with a null FILE * is undefined behavior, and
> doing so in practice will cause a SIGSEGV.  So it seems suitable for
> __nonnull.
> 
> This will help the compiler to warn for some buggy code, like
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=109570.

LGTM, thanks.

Reviewed-by: Adhemerval Zanella  <adhemerval.zanella@linaro.org>

> ---
>  libio/stdio.h | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/libio/stdio.h b/libio/stdio.h
> index 45ddafdf20..bfb8415d3b 100644
> --- a/libio/stdio.h
> +++ b/libio/stdio.h
> @@ -180,7 +180,7 @@ extern int renameat2 (int __oldfd, const char *__old, int __newfd,
>  
>     This function is a possible cancellation point and therefore not
>     marked with __THROW.  */
> -extern int fclose (FILE *__stream);
> +extern int fclose (FILE *__stream) __nonnull ((1));
>  
>  #undef __attr_dealloc_fclose
>  #define __attr_dealloc_fclose __attr_dealloc (fclose, 1)
> @@ -269,7 +269,7 @@ extern FILE *fopen (const char *__restrict __filename,
>     marked with __THROW.  */
>  extern FILE *freopen (const char *__restrict __filename,
>  		      const char *__restrict __modes,
> -		      FILE *__restrict __stream) __wur;
> +		      FILE *__restrict __stream) __wur __nonnull ((3));
>  #else
>  # ifdef __REDIRECT
>  extern FILE *__REDIRECT (fopen, (const char *__restrict __filename,
> @@ -290,7 +290,7 @@ extern FILE *fopen64 (const char *__restrict __filename,
>    __attribute_malloc__ __attr_dealloc_fclose __wur;
>  extern FILE *freopen64 (const char *__restrict __filename,
>  			const char *__restrict __modes,
> -			FILE *__restrict __stream) __wur;
> +			FILE *__restrict __stream) __wur __nonnull ((3));
>  #endif
>  
>  #ifdef	__USE_POSIX

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] libio: Add __nonnull for FILE * arguments of fclose and freopen
  2023-04-21 13:00 ` Adhemerval Zanella Netto
@ 2023-04-22  7:32   ` Xi Ruoyao
  0 siblings, 0 replies; 6+ messages in thread
From: Xi Ruoyao @ 2023-04-22  7:32 UTC (permalink / raw)
  To: Adhemerval Zanella Netto, libc-alpha

On Fri, 2023-04-21 at 10:00 -0300, Adhemerval Zanella Netto wrote:
> 
> 
> On 21/04/23 03:21, Xi Ruoyao wrote:
> > Calling fclose or freopen with a null FILE * is undefined behavior,
> > and
> > doing so in practice will cause a SIGSEGV.  So it seems suitable for
> > __nonnull.
> > 
> > This will help the compiler to warn for some buggy code, like
> > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=109570.
> 
> LGTM, thanks.

Thanks, please commit it if there is no objections.  Or may I get a
write after approve access for glibc.git?

> 
> Reviewed-by: Adhemerval Zanella  <adhemerval.zanella@linaro.org>
> 
> > ---
> >  libio/stdio.h | 6 +++---
> >  1 file changed, 3 insertions(+), 3 deletions(-)
> > 
> > diff --git a/libio/stdio.h b/libio/stdio.h
> > index 45ddafdf20..bfb8415d3b 100644
> > --- a/libio/stdio.h
> > +++ b/libio/stdio.h
> > @@ -180,7 +180,7 @@ extern int renameat2 (int __oldfd, const char
> > *__old, int __newfd,
> >  
> >     This function is a possible cancellation point and therefore not
> >     marked with __THROW.  */
> > -extern int fclose (FILE *__stream);
> > +extern int fclose (FILE *__stream) __nonnull ((1));
> >  
> >  #undef __attr_dealloc_fclose
> >  #define __attr_dealloc_fclose __attr_dealloc (fclose, 1)
> > @@ -269,7 +269,7 @@ extern FILE *fopen (const char *__restrict
> > __filename,
> >     marked with __THROW.  */
> >  extern FILE *freopen (const char *__restrict __filename,
> >                       const char *__restrict __modes,
> > -                     FILE *__restrict __stream) __wur;
> > +                     FILE *__restrict __stream) __wur __nonnull
> > ((3));
> >  #else
> >  # ifdef __REDIRECT
> >  extern FILE *__REDIRECT (fopen, (const char *__restrict __filename,
> > @@ -290,7 +290,7 @@ extern FILE *fopen64 (const char *__restrict
> > __filename,
> >    __attribute_malloc__ __attr_dealloc_fclose __wur;
> >  extern FILE *freopen64 (const char *__restrict __filename,
> >                         const char *__restrict __modes,
> > -                       FILE *__restrict __stream) __wur;
> > +                       FILE *__restrict __stream) __wur __nonnull
> > ((3));
> >  #endif
> >  
> >  #ifdef __USE_POSIX

-- 
Xi Ruoyao <xry111@xry111.site>
School of Aerospace Science and Technology, Xidian University

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Ping: [PATCH] libio: Add __nonnull for FILE * arguments of fclose and freopen
  2023-04-21  6:21 [PATCH] libio: Add __nonnull for FILE * arguments of fclose and freopen Xi Ruoyao
  2023-04-21 13:00 ` Adhemerval Zanella Netto
@ 2023-05-16  2:50 ` Xi Ruoyao
  2023-05-16 10:04   ` Adhemerval Zanella Netto
  1 sibling, 1 reply; 6+ messages in thread
From: Xi Ruoyao @ 2023-05-16  2:50 UTC (permalink / raw)
  To: libc-alpha; +Cc: Adhemerval Zanella Netto, Carlos O'Donell

Ping, as this seems reviewed as OK but fallen through two review
meetings. 

On Fri, 2023-04-21 at 14:21 +0800, Xi Ruoyao wrote:
> Calling fclose or freopen with a null FILE * is undefined behavior,
> and
> doing so in practice will cause a SIGSEGV.  So it seems suitable for
> __nonnull.
> 
> This will help the compiler to warn for some buggy code, like
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=109570.
> ---
>  libio/stdio.h | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/libio/stdio.h b/libio/stdio.h
> index 45ddafdf20..bfb8415d3b 100644
> --- a/libio/stdio.h
> +++ b/libio/stdio.h
> @@ -180,7 +180,7 @@ extern int renameat2 (int __oldfd, const char
> *__old, int __newfd,
>  
>     This function is a possible cancellation point and therefore not
>     marked with __THROW.  */
> -extern int fclose (FILE *__stream);
> +extern int fclose (FILE *__stream) __nonnull ((1));
>  
>  #undef __attr_dealloc_fclose
>  #define __attr_dealloc_fclose __attr_dealloc (fclose, 1)
> @@ -269,7 +269,7 @@ extern FILE *fopen (const char *__restrict
> __filename,
>     marked with __THROW.  */
>  extern FILE *freopen (const char *__restrict __filename,
>                       const char *__restrict __modes,
> -                     FILE *__restrict __stream) __wur;
> +                     FILE *__restrict __stream) __wur __nonnull
> ((3));
>  #else
>  # ifdef __REDIRECT
>  extern FILE *__REDIRECT (fopen, (const char *__restrict __filename,
> @@ -290,7 +290,7 @@ extern FILE *fopen64 (const char *__restrict
> __filename,
>    __attribute_malloc__ __attr_dealloc_fclose __wur;
>  extern FILE *freopen64 (const char *__restrict __filename,
>                         const char *__restrict __modes,
> -                       FILE *__restrict __stream) __wur;
> +                       FILE *__restrict __stream) __wur __nonnull
> ((3));
>  #endif
>  
>  #ifdef __USE_POSIX

-- 
Xi Ruoyao <xry111@xry111.site>
School of Aerospace Science and Technology, Xidian University

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: Ping: [PATCH] libio: Add __nonnull for FILE * arguments of fclose and freopen
  2023-05-16  2:50 ` Ping: " Xi Ruoyao
@ 2023-05-16 10:04   ` Adhemerval Zanella Netto
  2023-05-22 12:36     ` Carlos O'Donell
  0 siblings, 1 reply; 6+ messages in thread
From: Adhemerval Zanella Netto @ 2023-05-16 10:04 UTC (permalink / raw)
  To: Xi Ruoyao, libc-alpha; +Cc: Carlos O'Donell

Done.

On 15/05/23 23:50, Xi Ruoyao wrote:
> Ping, as this seems reviewed as OK but fallen through two review
> meetings. 
> 
> On Fri, 2023-04-21 at 14:21 +0800, Xi Ruoyao wrote:
>> Calling fclose or freopen with a null FILE * is undefined behavior,
>> and
>> doing so in practice will cause a SIGSEGV.  So it seems suitable for
>> __nonnull.
>>
>> This will help the compiler to warn for some buggy code, like
>> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=109570.
>> ---
>>  libio/stdio.h | 6 +++---
>>  1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/libio/stdio.h b/libio/stdio.h
>> index 45ddafdf20..bfb8415d3b 100644
>> --- a/libio/stdio.h
>> +++ b/libio/stdio.h
>> @@ -180,7 +180,7 @@ extern int renameat2 (int __oldfd, const char
>> *__old, int __newfd,
>>  
>>     This function is a possible cancellation point and therefore not
>>     marked with __THROW.  */
>> -extern int fclose (FILE *__stream);
>> +extern int fclose (FILE *__stream) __nonnull ((1));
>>  
>>  #undef __attr_dealloc_fclose
>>  #define __attr_dealloc_fclose __attr_dealloc (fclose, 1)
>> @@ -269,7 +269,7 @@ extern FILE *fopen (const char *__restrict
>> __filename,
>>     marked with __THROW.  */
>>  extern FILE *freopen (const char *__restrict __filename,
>>                       const char *__restrict __modes,
>> -                     FILE *__restrict __stream) __wur;
>> +                     FILE *__restrict __stream) __wur __nonnull
>> ((3));
>>  #else
>>  # ifdef __REDIRECT
>>  extern FILE *__REDIRECT (fopen, (const char *__restrict __filename,
>> @@ -290,7 +290,7 @@ extern FILE *fopen64 (const char *__restrict
>> __filename,
>>    __attribute_malloc__ __attr_dealloc_fclose __wur;
>>  extern FILE *freopen64 (const char *__restrict __filename,
>>                         const char *__restrict __modes,
>> -                       FILE *__restrict __stream) __wur;
>> +                       FILE *__restrict __stream) __wur __nonnull
>> ((3));
>>  #endif
>>  
>>  #ifdef __USE_POSIX
> 

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: Ping: [PATCH] libio: Add __nonnull for FILE * arguments of fclose and freopen
  2023-05-16 10:04   ` Adhemerval Zanella Netto
@ 2023-05-22 12:36     ` Carlos O'Donell
  0 siblings, 0 replies; 6+ messages in thread
From: Carlos O'Donell @ 2023-05-22 12:36 UTC (permalink / raw)
  To: Adhemerval Zanella Netto, Xi Ruoyao, libc-alpha

On 5/16/23 06:04, Adhemerval Zanella Netto wrote:
> Done.

Thanks for pushing this last week!

> On 15/05/23 23:50, Xi Ruoyao wrote:
>> Ping, as this seems reviewed as OK but fallen through two review
>> meetings. 
>>
>> On Fri, 2023-04-21 at 14:21 +0800, Xi Ruoyao wrote:
>>> Calling fclose or freopen with a null FILE * is undefined behavior,
>>> and
>>> doing so in practice will cause a SIGSEGV.  So it seems suitable for
>>> __nonnull.
>>>
>>> This will help the compiler to warn for some buggy code, like
>>> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=109570.
>>> ---
>>>  libio/stdio.h | 6 +++---
>>>  1 file changed, 3 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/libio/stdio.h b/libio/stdio.h
>>> index 45ddafdf20..bfb8415d3b 100644
>>> --- a/libio/stdio.h
>>> +++ b/libio/stdio.h
>>> @@ -180,7 +180,7 @@ extern int renameat2 (int __oldfd, const char
>>> *__old, int __newfd,
>>>  
>>>     This function is a possible cancellation point and therefore not
>>>     marked with __THROW.  */
>>> -extern int fclose (FILE *__stream);
>>> +extern int fclose (FILE *__stream) __nonnull ((1));
>>>  
>>>  #undef __attr_dealloc_fclose
>>>  #define __attr_dealloc_fclose __attr_dealloc (fclose, 1)
>>> @@ -269,7 +269,7 @@ extern FILE *fopen (const char *__restrict
>>> __filename,
>>>     marked with __THROW.  */
>>>  extern FILE *freopen (const char *__restrict __filename,
>>>                       const char *__restrict __modes,
>>> -                     FILE *__restrict __stream) __wur;
>>> +                     FILE *__restrict __stream) __wur __nonnull
>>> ((3));
>>>  #else
>>>  # ifdef __REDIRECT
>>>  extern FILE *__REDIRECT (fopen, (const char *__restrict __filename,
>>> @@ -290,7 +290,7 @@ extern FILE *fopen64 (const char *__restrict
>>> __filename,
>>>    __attribute_malloc__ __attr_dealloc_fclose __wur;
>>>  extern FILE *freopen64 (const char *__restrict __filename,
>>>                         const char *__restrict __modes,
>>> -                       FILE *__restrict __stream) __wur;
>>> +                       FILE *__restrict __stream) __wur __nonnull
>>> ((3));
>>>  #endif
>>>  
>>>  #ifdef __USE_POSIX
>>
> 

-- 
Cheers,
Carlos.


^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2023-05-22 12:36 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-04-21  6:21 [PATCH] libio: Add __nonnull for FILE * arguments of fclose and freopen Xi Ruoyao
2023-04-21 13:00 ` Adhemerval Zanella Netto
2023-04-22  7:32   ` Xi Ruoyao
2023-05-16  2:50 ` Ping: " Xi Ruoyao
2023-05-16 10:04   ` Adhemerval Zanella Netto
2023-05-22 12:36     ` Carlos O'Donell

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).