closedir(3) handling NULL

[Bryan Drewery]

On Sat, Jan 25, 2014 at 06:00:08AM +1100, Bruce Evans wrote:
> On Fri, 24 Jan 2014, Bryan Drewery wrote:
> 
> > On Fri, Jan 24, 2014 at 02:24:35PM +0100, Jilles Tjoelker wrote:
> >> On Thu, Jan 23, 2014 at 07:41:05PM -0600, Bryan Drewery wrote:
> >>> I found that Linux handles closedir(NULL) fine and returns EINVAL. POSIX
> >>> [1] specifies that EBADF should be returned if "The dirp argument does
> >>> not refer to an open directory stream"
> >>
> >>> [1] http://pubs.opengroup.org/onlinepubs/009696799/functions/closedir.html
> 
> > I do think that improving portability is important. Even against sloppy
> > coding. Applications developed for Linux are fine passing NULL to closedir(3),
> > which leads to a style of coding that does not reveal itself to be a
> > problem on FreeBSD until an edge case comes up.
> 
> This unimproves portability.  FreeBSD intentionally does the opposite for
> fclose(): from fclose(3):

IMHO we should handle NULL gracefully in all places instead of having
hidden surprises.

> 
> @ NOTES
> @      The fclose() function does not handle NULL arguments; they will result in
> @      a segmentation violation.  This is intentional - it makes it easier to
> @      make sure programs written under FreeBSD are bug free.  This behaviour is
> @      an implementation detail, and programs should not rely upon it.
> 
> It would be good to do the same thing for garbage stream pointers.
[...]
> % diff --git lib/libc/gen/closedir.c lib/libc/gen/closedir.c
> % index 88ded37..d7a5bdb 100644
> % --- lib/libc/gen/closedir.c
> % +++ lib/libc/gen/closedir.c
> % @@ -53,6 +53,11 @@ fdclosedir(DIR *dirp)
> %  {
> %  	int fd;
> % 
> % +	if (dirp == NULL) {
> % +		errno = EBADF;
> % +		return (-1);
> % +	}
> % +
> 
> Style bug (extra blank line).
> 
> %  	if (__isthreaded)
> %  		_pthread_mutex_lock(&dirp->dd_lock);
> 
> Example of normal style (no extra blank line after an if statement).
> 
> Extra blank lines are especially not needed after return statements since
> return statements obviously don't fall through.
> 
> %  	fd = dirp->dd_fd;
> % @@ -71,6 +76,10 @@ fdclosedir(DIR *dirp)
> %  int
> %  closedir(DIR *dirp)
> %  {
> % +	int fd;
> % +
> % +	if ((fd = fdclosedir(dirp)) == -1)
> % +		return (-1);
> %
> 
> Style bug (extra blank line).
> 
> There is no man page for fdclosedir(3).  It is in directory(3), but someone
> forgot to add fdclosedir.3 to MLINKS.  fdopendir.3 is unsorted in MLINKS
> instead of missing.

Thanks for the style comments. I am now aware.

I am fixing the MLINKS issues.

> 
> % -	return (_close(fdclosedir(dirp)));
> % +	return (_close(fd));
> %  }


Bryan Drewery
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 964 bytes
Desc: not available
URL: <http://lists.freebsd.org/pipermail/freebsd-standards/attachments/20140124/ab11470f/attachment.sig>