Skip site navigation (1)Skip section navigation (2)
Date:      Tue, 18 Mar 2014 22:35:16 +0100
From:      Jilles Tjoelker <jilles@stack.nl>
To:        Mariusz Zaborski <oshogbo@FreeBSD.org>
Cc:        freebsd-current@freebsd.org, freebsd-arch@freebsd.org
Subject:   Re: Hello fdclose
Message-ID:  <20140318213516.GA71491@stack.nl>
In-Reply-To: <CAGOYWV80vTTQbvSjvNa6XBzBiKy%2BjnGantkUH_RO=8prxoHmyQ@mail.gmail.com>
References:  <CAGOYWV80vTTQbvSjvNa6XBzBiKy%2BjnGantkUH_RO=8prxoHmyQ@mail.gmail.com>

next in thread | previous in thread | raw e-mail | index | archive | help
On Tue, Mar 18, 2014 at 12:23:19AM +0100, Mariusz Zaborski wrote:
> After our previous discuss  [1] I prepare fdclosedir(3) function which
> was committed by Pawel (cc'ed) in commit r254499.

> A while ago I also prepare the fdclose function. Unfortunately, this
> new function is a little bit more tricky then previous one. Can I ask
> you for a review of this patch?

Does this patch allow perl to stop writing to FILE._file? As pointed out
in
http://lists.freebsd.org/pipermail/freebsd-current/2013-January/039024.html
perlio.c in the perl source contains a function
PerlIOStdio_invalidate_fileno() that should modify a FILE such that
fclose() does not close the file descriptor but still frees all memory
(Perl has already called fflush()). Although using fdclose() could solve
this without touching the internals of FILE, this will make perlio.c
uglier with even more #ifdefs.

> [snip]
> --- //depot/user/oshogbo/capsicum/lib/libc/stdio/Symbol.map	2013-06-28 08:51:28.000000000 0000
> +++ /home/oshogbo/p4/capsicum/lib/libc/stdio/Symbol.map	2013-06-28 08:51:28.000000000 0000
> @@ -156,6 +156,7 @@
>  	putwc_l;
>  	putwchar_l;
>  	fmemopen;
> +	fdclose;
>  	open_memstream;
>  	open_wmemstream;
>  };

This should be in the FBSD_1.4 namespace (which does not exist yet).

> [snip]
> --- //depot/user/oshogbo/capsicum/lib/libc/stdio/fclose.c	2013-06-28 08:51:28.000000000 0000
> +++ /home/oshogbo/p4/capsicum/lib/libc/stdio/fclose.c	2013-06-28 08:51:28.000000000 0000
> [snip]
> +int
> +fdclose(FILE *fp)
> +{
> +	int fd, r, err;
> +
> +	if (fp->_flags == 0) {	/* not open! */
> +		errno = EBADF;
> +		return (EOF);
> +	}
> +
> +	r = 0;
> +	FLOCKFILE(fp);
> +	fd = fp->_file;
> +	if (fp->_close != __sclose) {
> +		r = EOF;
> +		errno = EOPNOTSUPP;
> +	} else if (fd < 0) {
> +		r = EOF;
> +		errno = EBADF;
> +	}
> +	if (r == EOF) {
> +		err = errno;
> +		(void)cleanfile(fp, true);
> +		errno = err;
> +	} else {
> +		r = cleanfile(fp, false);
> +	}
>  	FUNLOCKFILE(fp);
> +
> +	return (r == 0 ? fd : r);

If a file descriptor would be returned but cleanfile() returns an error
(e.g. write error on flush), the file descriptor is not returned and not
closed.

I think that in cases where fdclose() would be used, it is essential
that the file descriptor is never closed. This means that the API needs
to be different so it can report a write error but still return a file
descriptor. One way to do this is to return the file descriptor by
reference. Another is to expect the application to call fileno() and not
return the descriptor from the new function.

-- 
Jilles Tjoelker



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20140318213516.GA71491>