| Message ID | 20230421062133.2084910-1-xry111@xry111.site |
|---|---|
| State | New |
| Headers | show |
| Series | libio: Add __nonnull for FILE * arguments of fclose and freopen | expand |
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
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
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
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 >
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 >> >
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