[man-pages PATCH] Add note about calling fclose(NULL)

[man-pages PATCH] Add note about calling fclose(NULL)

[Thomas Jarosch]

Signed-off-by: Thomas Jarosch <thomas.jarosch-XXsH3GEs1jrby3iVrkZq2A@public.gmane.org>
---
 man3/fclose.3 | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/man3/fclose.3 b/man3/fclose.3
index 1bdf0a5..6945e89 100644
--- a/man3/fclose.3
+++ b/man3/fclose.3
@@ -97,6 +97,10 @@ on disk the kernel buffers must be flushed too, for example, with
 .BR sync (2)
 or
 .BR fsync (2).
+Also it's undefined behavior to call
+.BR fclose ()
+with a NULL pointer.
+
 .SH "SEE ALSO"
 .BR close (2),
 .BR fcloseall (3),
-- 
1.7.11.7

--
To unsubscribe from this list: send the line "unsubscribe linux-man" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [man-pages PATCH] Add note about calling fclose(NULL)

[Mike Frysinger]

[-- Attachment #1: Type: Text/Plain, Size: 439 bytes --]

On Tuesday 29 January 2013 11:24:43 Thomas Jarosch wrote:
> --- a/man3/fclose.3
> +++ b/man3/fclose.3
> @@ -97,6 +97,10 @@ on disk the kernel buffers must be flushed too, for
> example, with .BR sync (2)
>  or
>  .BR fsync (2).
> +Also it's undefined behavior to call
> +.BR fclose ()
> +with a NULL pointer.
> +
>  .SH "SEE ALSO"
>  .BR close (2),
>  .BR fcloseall (3),

i wonder if it'd be better placed in the DESCRIPTION section
-mike

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [man-pages PATCH] Add note about calling fclose(NULL)

[Michael Kerrisk (man-pages)]

Thomas,

Could you supply some explanation of why you think this patch is required?

Thanks,

Michael

On Tue, Jan 29, 2013 at 5:24 PM, Thomas Jarosch
<thomas.jarosch-XXsH3GEs1jrby3iVrkZq2A@public.gmane.org> wrote:
>
> Signed-off-by: Thomas Jarosch <thomas.jarosch-XXsH3GEs1jrby3iVrkZq2A@public.gmane.org>
> ---
>  man3/fclose.3 | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/man3/fclose.3 b/man3/fclose.3
> index 1bdf0a5..6945e89 100644
> --- a/man3/fclose.3
> +++ b/man3/fclose.3
> @@ -97,6 +97,10 @@ on disk the kernel buffers must be flushed too, for example, with
>  .BR sync (2)
>  or
>  .BR fsync (2).
> +Also it's undefined behavior to call
> +.BR fclose ()
> +with a NULL pointer.
> +
>  .SH "SEE ALSO"
>  .BR close (2),
>  .BR fcloseall (3),
> --
> 1.7.11.7
>



-- 
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Author of "The Linux Programming Interface"; http://man7.org/tlpi/
--
To unsubscribe from this list: send the line "unsubscribe linux-man" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [man-pages PATCH] Add note about calling fclose(NULL)

[Thomas Jarosch]

On Thursday, 31. January 2013 01:50:49 Michael Kerrisk wrote:
> > diff --git a/man3/fclose.3 b/man3/fclose.3
> > index 1bdf0a5..6945e89 100644
> > --- a/man3/fclose.3
> > +++ b/man3/fclose.3
> > @@ -97,6 +97,10 @@ on disk the kernel buffers must be flushed too, for
> > example, with> 
> >  .BR sync (2)
> >  or
> >  .BR fsync (2).
> > 
> > +Also it's undefined behavior to call
> > +.BR fclose ()
> > +with a NULL pointer.
> > +
>
> Could you supply some explanation of why you think this patch is required?

I always thought fclose(NULL) behaves like free(NULL)
and even sent a patch for the "perf" utility assuming this.
Luckily Arnaldo caught the error before applying the patch,
at least in one case.

Also fclose(NULL) "works" fine on HP-UX and Solaris,
glibc will die with a segfault.

Cheers,
Thomas

--
To unsubscribe from this list: send the line "unsubscribe linux-man" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [man-pages PATCH] Add note about calling fclose(NULL)

[Thomas Jarosch]

Hi Mike,

On Tuesday, 29. January 2013 13:44:28 you wrote:
> On Tuesday 29 January 2013 11:24:43 Thomas Jarosch wrote:
> > --- a/man3/fclose.3
> > +++ b/man3/fclose.3
> > @@ -97,6 +97,10 @@ on disk the kernel buffers must be flushed too, for
> > example, with .BR sync (2)
> > 
> >  or
> >  .BR fsync (2).
> > 
> > +Also it's undefined behavior to call
> > +.BR fclose ()
> > +with a NULL pointer.
> > +
> > 
> >  .SH "SEE ALSO"
> >  .BR close (2),
> >  .BR fcloseall (3),
> 
> i wonder if it'd be better placed in the DESCRIPTION section

I first added it there and then thought it's too prominent.
The "NOTES" section seemed like a better place.

Let's see if it gets accepted at all.

Thomas

--
To unsubscribe from this list: send the line "unsubscribe linux-man" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [man-pages PATCH] Add note about calling fclose(NULL)

[Michael Kerrisk]

On 01/31/2013 09:26 AM, Thomas Jarosch wrote:
> On Thursday, 31. January 2013 01:50:49 Michael Kerrisk wrote:
>>> diff --git a/man3/fclose.3 b/man3/fclose.3
>>> index 1bdf0a5..6945e89 100644
>>> --- a/man3/fclose.3
>>> +++ b/man3/fclose.3
>>> @@ -97,6 +97,10 @@ on disk the kernel buffers must be flushed too, for
>>> example, with> 
>>>  .BR sync (2)
>>>  or
>>>  .BR fsync (2).
>>>
>>> +Also it's undefined behavior to call
>>> +.BR fclose ()
>>> +with a NULL pointer.
>>> +
>>
>> Could you supply some explanation of why you think this patch is required?
> 
> I always thought fclose(NULL) behaves like free(NULL)
> and even sent a patch for the "perf" utility assuming this.
> Luckily Arnaldo caught the error before applying the patch,
> at least in one case.
> 
> Also fclose(NULL) "works" fine on HP-UX and Solaris,
> glibc will die with a segfault.

Thomas,

Can you define "works" with respect to those OSes? Their man pages seems 
to say nothing about fclose(NULL). At this stage, I'm not convinced that
adding this text to the man page is really required.

Thanks,

Michael


--
To unsubscribe from this list: send the line "unsubscribe linux-man" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [man-pages PATCH] Add note about calling fclose(NULL)

[Mike Frysinger]

[-- Attachment #1: Type: Text/Plain, Size: 1018 bytes --]

On Friday 01 February 2013 04:43:21 Thomas Jarosch wrote:
> On Tuesday, 29. January 2013 13:44:28 you wrote:
> > On Tuesday 29 January 2013 11:24:43 Thomas Jarosch wrote:
> > > --- a/man3/fclose.3
> > > +++ b/man3/fclose.3
> > > @@ -97,6 +97,10 @@ on disk the kernel buffers must be flushed too, for
> > > example, with .BR sync (2)
> > >  or
> > >  .BR fsync (2).
> > > +Also it's undefined behavior to call
> > > +.BR fclose ()
> > > +with a NULL pointer.
> > > +
> > >  .SH "SEE ALSO"
> > >  .BR close (2),
> > >  .BR fcloseall (3),
> > 
> > i wonder if it'd be better placed in the DESCRIPTION section
> 
> I first added it there and then thought it's too prominent.
> The "NOTES" section seemed like a better place.

looking at other pages, argument behavior is in the DESCRIPTION rather than 
NOTES, including discussion of undefined behavior.  keeping it higher up is 
probably better too since NOTES tends to get lost by readers while DESCRIPTION 
is absorbed most frequently.
-mike

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 836 bytes --]