Details

Reviewers

jilles
jhb
allanjude
wblock
pjd
bdrewery

Group Reviewers

manpages

Commits

rS292154: MFstable/10 r291602:
rS291602: MFC r285140:
rS285140: Add fdclose(3) function.

Summary

After a quite long time I would like finally finish this function. :)
The change to the perl will be very simple (I believe there is no way to mix src and ports repositories in phabricator):

--- perl-5.20.2/perlio.c
+++ perl-5.20.2/perlio.c
@@ -3122,12 +3122,7 @@
     f->_file = -1;
     return 1;
 #  elif defined(__FreeBSD__)
-    /* There may be a better way on FreeBSD:
-        - we could insert a dummy func in the _close function entry
-	f->_close = (int (*)(void *)) dummy_close;
-     */
-    f->_file = -1;
-    return 1;
+    return fdclose(f, NULL) == 0 ? 1 : 0;
 #  elif defined(__OpenBSD__)
     /* There may be a better way on OpenBSD:
         - we could insert a dummy func in the _close function entry

Diff Detail

Lint

Lint Skipped

Unit

Tests Skipped

Event Timeline

oshogbo updated this revision to Diff 5843.May 31 2015, 7:14 PM

oshogbo retitled this revision from to fdclose.

oshogbo updated this object.

oshogbo edited the test plan for this revision. (Show Details)

oshogbo added reviewers: pjd, jilles, jhb, bdrewery, allanjude.

oshogbo set the repository for this revision to rS FreeBSD src repository - subversion.

Herald added a reviewer: manpages. · View Herald TranscriptMay 31 2015, 7:14 PM

Herald added a subscriber: imp. · View Herald Transcript

The code looks good to me.

lib/libc/stdio/fclose.3
72	Calling the argument fdp instead of fd may make this clearer. Try: If .Fa fd is not .Dv NULL , the file descriptor will be written to it.
73	The words "non-standard close method" may be unclear. Perhaps say "If the stream does not have an associated file descriptor", possibly with examples like fmemopen(), funopen() and open_memstream(). These words also occur under ERRORS below. If the stream is not open, the behaviour is not defined and need not be described in the man page.
105–106	This change seems unnecessary. It is still the case that there are two situations: success and error.
122	errno should keep its .Va markup
151	This should be `.Sh HISTORY`.

Update man page as jilles suggest.

wblock added a subscriber: wblock.Jun 1 2015, 5:57 PM

wblock added inline comments.

lib/libc/stdio/fclose.3
78	Needs a comma after "descriptor".
81	"Such stream" should be "Such a stream", but it's a little confusing, maybe "This type of stream", or maybe it does not even need to be said. "functions like" is confusing, probably should be "such as". Finally, switch to active rather than passive voice by changing "could" to "can" or "is": Streams are created with functions such as or This type of stream is created with functions such as or even This type of stream is created with
95	Several things: Not necessary to use "the blah function". After the first time, just say the name of the function: "blah returns..." (no leading "The"). "returns no value" is somewhat ambiguous, could be misread to mean it returns null. "does not return a value" might be more clear. So: .Fn fcloseall does not return a value.
109	"Shall" implies a recommendation. Probably better as "will".
135	(Missing an "it" after "makes"). Use a full stop rather than an emdash: This is intentional. It makes it easier to make sure programs written under

The code changes look fine to me (I'll defer to others on the manpage bits).

This revision is now accepted and ready to land.Jun 1 2015, 6:22 PM

Update man page as wblock suggest. Thanks!

This revision now requires review to proceed.Jun 1 2015, 6:57 PM

@wblock

"Shall" implies a recommendation. Probably better as "will".

I see this is a problematic one.
In first version i had "will" but Bruce suggest me that I should changed it.

I don't like the tense given by "will" in man pages. POSIX says "shall
fail" in similar contexts, and "will fail" is a mistranslation of this
("shall" is a technical term that doesn't suggest future tense).

Missed one suggested by @wblock.

wblock added inline comments.Jun 2 2015, 5:12 PM

lib/libc/stdio/fclose.3
38	No comma after "descriptor" here. (This might be due to phabricator "helping" by mixing up diffs. I'll try to include some context to make this easier.) 38 the file descriptor will be written to it.
41	Typo: s/arguemnt/argument/
42	Needs a comma after this "descriptor", where the pause is located: 42 If the stream does not have an associated file descriptor,
99	Start the new sentence on a new line: 99 This is intentional. 100 It makes it easier to make sure programs written under

In D2697#51201, @oshogbo wrote:

@wblock

"Shall" implies a recommendation. Probably better as "will".

I see this is a problematic one.
In first version i had "will" but Bruce suggest me that I should changed it.

I don't like the tense given by "will" in man pages. POSIX says "shall
fail" in similar contexts, and "will fail" is a mistranslation of this
("shall" is a technical term that doesn't suggest future tense).

All right, let's eliminate the stilted language altogether:

Original:

The
.Fn fdclose
function shall fail if:

New:

.Fn fdclose
fails if:

wblock suggestions.

I also add missing .El.

Compromise about shall and will :)

wblock added inline comments.Jun 2 2015, 9:47 PM

lib/libc/stdio/fclose.3
41	Oops, I missed the other typo: s/diffrent/different/

s/diffrent/different/

Man page looks good. Please test with igor -R fclose.3 | less -RS and mandoc -Tlint fclose.3 before commit.

This revision is now accepted and ready to land.Jun 3 2015, 3:27 PM

Closed by commit rS285140: Add fdclose(3) function. (authored by oshogbo). · Explain WhyJul 4 2015, 4:42 PM

This revision was automatically updated to reflect the committed changes.

oshogbo marked an inline comment as done.

Diff 5869

View Options

include/stdio.h

Context not available.
	int asprintf(char *, const char , ...) __printflike(2, 3);	int asprintf(char *, const char , ...) __printflike(2, 3);
	char ctermid_r(char );	char ctermid_r(char );
	void fcloseall(void);	void fcloseall(void);
		int fdclose(FILE , int );
	char fgetln(FILE , size_t *);	char fgetln(FILE , size_t *);
	const char fmtcheck(const char , const char *) __format_arg(2);	const char fmtcheck(const char , const char *) __format_arg(2);
	int fpurge(FILE *);	int fpurge(FILE *);
Context not available.

View Options

lib/libc/stdio/Symbol.map

Context not available.
	mkostemps;	mkostemps;
	};	};

		FBSD_1.4 {
		fdclose;
		};

	FBSDprivate_1.0 {	FBSDprivate_1.0 {
	_flockfile;	_flockfile;
	_flockfile_debug_stub;	_flockfile_debug_stub;
Context not available.

View Options

lib/libc/stdio/fclose.3

	.\" Copyright (c) 1990, 1991, 1993	.\" Copyright (c) 1990, 1991, 1993 The Regents of the University of California.
	.\" The Regents of the University of California. All rights reserved.	.\" Copyright (c) 2015 Mariusz Zaborski <oshogbo@FreeBSD.org>
		.\" All rights reserved.
	.\"	.\"
	.\" This code is derived from software contributed to Berkeley by	.\" This code is derived from software contributed to Berkeley by
	.\" Chris Torek and the American National Standards Committee X3,	.\" Chris Torek and the American National Standards Committee X3,
Context not available.
	.\" @(#)fclose.3 8.1 (Berkeley) 6/4/93	.\" @(#)fclose.3 8.1 (Berkeley) 6/4/93
	.\" $FreeBSD$	.\" $FreeBSD$
	.\"	.\"
	.Dd April 22, 2006	.Dd Jun 1, 2015
	.Dt FCLOSE 3	.Dt FCLOSE 3
	.Os	.Os
		wblockUnsubmitted Done Inline Actions No comma after "descriptor" here. (This might be due to phabricator "helping" by mixing up diffs. I'll try to include some context to make this easier.) 38 the file descriptor will be written to it. wblock: No comma after "descriptor" here. (This might be due to phabricator "helping" by mixing up…
	.Sh NAME	.Sh NAME
	.Nm fclose ,	.Nm fclose ,
		.Nm fdclose ,
		wblockUnsubmitted Done Inline Actions Typo: s/arguemnt/argument/ wblock: Typo: s/arguemnt/argument/
		wblockUnsubmitted Done Inline Actions Oops, I missed the other typo: s/diffrent/different/ wblock: Oops, I missed the other typo: s/diffrent/different/
	.Nm fcloseall	.Nm fcloseall
		wblockUnsubmitted Done Inline Actions Needs a comma after this "descriptor", where the pause is located: 42 If the stream does not have an associated file descriptor, wblock: Needs a comma after this "descriptor", where the pause is located: 42 If the stream does not…
	.Nd close a stream	.Nd close a stream
	.Sh LIBRARY	.Sh LIBRARY
Context not available.
	.In stdio.h	.In stdio.h
	.Ft int	.Ft int
	.Fn fclose "FILE *stream"	.Fn fclose "FILE *stream"
		.Ft int
		.Fn fdclose "FILE stream" "int fdp"
	.Ft void	.Ft void
	.Fn fcloseall void	.Fn fcloseall void
	.Sh DESCRIPTION	.Sh DESCRIPTION
Context not available.
	.Xr fflush 3 .	.Xr fflush 3 .
	.Pp	.Pp
	The	The
		.Fn fdclose
		function is equivalent to
		.Fn fclose
		except that it does not close the underlying file descriptor.
		If
		.Fa fdp
		is not
		jillesUnsubmitted Done Inline Actions Calling the argument fdp instead of fd may make this clearer. Try: If .Fa fd is not .Dv NULL , the file descriptor will be written to it. jilles: Calling the argument fdp instead of fd may make this clearer. Try: If .Fa fd is not .Dv NULL…
		.Dv NULL ,
		jillesUnsubmitted Done Inline Actions The words "non-standard close method" may be unclear. Perhaps say "If the stream does not have an associated file descriptor", possibly with examples like fmemopen(), funopen() and open_memstream(). These words also occur under ERRORS below. If the stream is not open, the behaviour is not defined and need not be described in the man page. jilles: The words "non-standard close method" may be unclear. Perhaps say "If the stream does not have…
		the file descriptor, will be written to it.
		If the
		.Fa fdp
		arguemnt will be diffrent then NULL the file descriptor will be returned in it,
		If the stream does not have an associated file descriptor
		wblockUnsubmitted Done Inline Actions Needs a comma after "descriptor". wblock: Needs a comma after "descriptor".
		.Fa fdp
		will be set to -1.
		This type of stream is created with functions such as
		wblockUnsubmitted Done Inline Actions "Such stream" should be "Such a stream", but it's a little confusing, maybe "This type of stream", or maybe it does not even need to be said. "functions like" is confusing, probably should be "such as". Finally, switch to active rather than passive voice by changing "could" to "can" or "is": Streams are created with functions such as or This type of stream is created with functions such as or even This type of stream is created with wblock: "Such stream" should be "Such a stream", but it's a little confusing, maybe "This type of…
		.Xr fmemopen 3 ,
		.Xr funopen 3 ,
		or
		.Xr open_memstream 3 .
		.Pp
		The
	.Fn fcloseall	.Fn fcloseall
	function calls	function calls
	.Fn fclose	.Fn fclose
	on all open streams.	on all open streams.
	.Sh RETURN VALUES	.Sh RETURN VALUES
	Upon successful completion 0 is returned.	.Fn fcloseall
		returns no value.
		.Pp
		wblockUnsubmitted Done Inline Actions Several things: Not necessary to use "the blah function". After the first time, just say the name of the function: "blah returns..." (no leading "The"). "returns no value" is somewhat ambiguous, could be misread to mean it returns null. "does not return a value" might be more clear. So: .Fn fcloseall does not return a value. wblock: Several things: 1. Not necessary to use "the blah function". After the first time, just say…
		Upon successful completion the
		.Fn fclose
		and
		.Fn fdclose
		wblockUnsubmitted Done Inline Actions Start the new sentence on a new line: 99 This is intentional. 100 It makes it easier to make sure programs written under wblock: Start the new sentence on a new line: 99 This is intentional. 100 It makes it easier to…
		functions return 0.
	Otherwise,	Otherwise,
	.Dv EOF	.Dv EOF
	is returned and the global variable	is returned and the global variable
	.Va errno	.Va errno
	is set to indicate the error.	is set to indicate the error.
	In either case no further access to the stream is possible.
	.Sh ERRORS	.Sh ERRORS
		jillesUnsubmitted Done Inline Actions This change seems unnecessary. It is still the case that there are two situations: success and error. jilles: This change seems unnecessary. It is still the case that there are two situations: success and…
	The	The
		.Fn fdclose
		function shall fail if:
		wblockUnsubmitted Done Inline Actions "Shall" implies a recommendation. Probably better as "will". wblock: "Shall" implies a recommendation. Probably better as "will".
		.Bl -tag -width Er
		.It Bq Er EOPNOTSUPP
		The stream does not have an associated file descriptor.
		.Pp
		The
	.Fn fclose	.Fn fclose
	function	and
	may also fail and set	.Fn fdclose
		functions may also fail and set
	.Va errno	.Va errno
	for any of the errors specified for the routines	for any of the errors specified for
	.Xr close 2
	or
	.Xr fflush 3 .	.Xr fflush 3 .
		.Pp
		jillesUnsubmitted Done Inline Actions errno should keep its .Va markup jilles: errno should keep its .Va markup
		The
		.Fn fclose
		function may also fail and set errno for any of the errors specified for
		.Xr close 2.
	.Sh NOTES	.Sh NOTES
	The	The
	.Fn fclose	.Fn fclose
	function	and
	does not handle NULL arguments; they will result in a segmentation	.Fn fdclose
	violation.	functions do not handle NULL arguments in the
	This is intentional - it makes it easier to make sure programs written	.Fa stream
	under	variable; this will result in a segmentation violation.
		This is intentional. It makes it easier to make sure programs written under
		wblockUnsubmitted Done Inline Actions (Missing an "it" after "makes"). Use a full stop rather than an emdash: This is intentional. It makes it easier to make sure programs written under wblock: (Missing an "it" after "makes"). Use a full stop rather than an emdash: This is intentional.
	.Fx	.Fx
	are bug free.	are bug free.
	This behaviour is an implementation detail, and programs should not	This behaviour is an implementation detail, and programs should not
Context not available.
	function	function
	conforms to	conforms to
	.St -isoC .	.St -isoC .
	.Pp	.Sh HISTORY
		jillesUnsubmitted Done Inline Actions This should be `.Sh HISTORY`. jilles: This should be `.Sh HISTORY`.
	The	The
	.Fn fcloseall	.Fn fcloseall
	function first appeared in	function first appeared in
	.Fx 7.0 .	.Fx 7.0 .
		.Pp
		The
		.Fn fdclose
		function first appeared in
		.Fx 11.0 .
Context not available.

View Options

lib/libc/stdio/fclose.c

	/*-	/*-
	* Copyright (c) 1990, 1993	* Copyright (c) 1990, 1993 The Regents of the University of California.
	* The Regents of the University of California. All rights reserved.	* Copyright (c) 2013 Mariusz Zaborski <oshogbo@FreeBSD.org>
		* All rights reserved.
	*	*
	* This code is derived from software contributed to Berkeley by	* This code is derived from software contributed to Berkeley by
	* Chris Torek.	* Chris Torek.
Context not available.

	#include "namespace.h"	#include "namespace.h"
	#include <errno.h>	#include <errno.h>
		#include <stdbool.h>
	#include <stdio.h>	#include <stdio.h>
	#include <stdlib.h>	#include <stdlib.h>
	#include "un-namespace.h"	#include "un-namespace.h"
Context not available.
	#include "libc_private.h"	#include "libc_private.h"
	#include "local.h"	#include "local.h"

	int	static int
	fclose(FILE *fp)	cleanfile(FILE *fp, bool c)
	{	{
	int r;	int r;

	if (fp->_flags == 0) { /* not open! */
	errno = EBADF;
	return (EOF);
	}
	FLOCKFILE(fp);
	r = fp->_flags & __SWR ? __sflush(fp) : 0;	r = fp->_flags & __SWR ? __sflush(fp) : 0;
	if (fp->_close != NULL && (*fp->_close)(fp->_cookie) < 0)	if (c) {
	r = EOF;	if (fp->_close != NULL && (*fp->_close)(fp->_cookie) < 0)
		r = EOF;
		}

	if (fp->_flags & __SMBF)	if (fp->_flags & __SMBF)
	free((char *)fp->_bf._base);	free((char *)fp->_bf._base);
	if (HASUB(fp))	if (HASUB(fp))
Context not available.
	STDIO_THREAD_LOCK();	STDIO_THREAD_LOCK();
	fp->_flags = 0; /* Release this FILE for reuse. */	fp->_flags = 0; /* Release this FILE for reuse. */
	STDIO_THREAD_UNLOCK();	STDIO_THREAD_UNLOCK();

		return (r);
		}

		int
		fdclose(FILE fp, int fdp)
		{
		int r, err;

		if (fdp != NULL)
		*fdp = -1;

		if (fp->_flags == 0) { /* not open! */
		errno = EBADF;
		return (EOF);
		}

		FLOCKFILE(fp);
		r = 0;
		if (fp->_close != __sclose) {
		r = EOF;
		errno = EOPNOTSUPP;
		} else if (fp->_file < 0) {
		r = EOF;
		errno = EBADF;
		}
		if (r == EOF) {
		err = errno;
		(void)cleanfile(fp, true);
		errno = err;
		} else {
		if (fdp != NULL)
		*fdp = fp->_file;
		r = cleanfile(fp, false);
		}
	FUNLOCKFILE(fp);	FUNLOCKFILE(fp);

		return (r);
		}

		int
		fclose(FILE *fp)
		{
		int r;

		if (fp->_flags == 0) { /* not open! */
		errno = EBADF;
		return (EOF);
		}

		FLOCKFILE(fp);
		r = cleanfile(fp, true);
		FUNLOCKFILE(fp);

	return (r);	return (r);
	}	}
Context not available.

fdclose
ClosedPublic
Actions

Details

Diff Detail

Event Timeline

Revision Contents
Changeset List

Diff 5869

include/stdio.h

lib/libc/stdio/Symbol.map

lib/libc/stdio/fclose.3

lib/libc/stdio/fclose.c

fdcloseClosedPublicActions

Details

Diff Detail

Event Timeline

Revision ContentsChangeset List

Diff 5869

include/stdio.h

lib/libc/stdio/Symbol.map

lib/libc/stdio/fclose.3

lib/libc/stdio/fclose.c

fdclose
ClosedPublic
Actions

Revision Contents
Changeset List