Re: Hello fdclose

On Monday, March 17, 2014 7:23:19 pm Mariusz Zaborski wrote:
> Hi,
> 
> 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?

I think the code is fine.  I have a few suggestions on the manpage wording:

 The
+.Fn fdclose
+function is equivalent to the
+.Fn fclose
+function except that this function returns file descriptor instead of
+closing it.
+.Pp
+The

I would move fdclose() to its own paragraph and reword this sentence as:

  "The fdclose() function is equivalent to fclose() except that it does
   not close the underlying file descriptor."

 .Sh RETURN VALUES
-Upon successful completion 0 is returned.
+The
+.Fn fcloseall
+function return no value.
+.Pp
+Upon successful completion
+.Fn fclose
+return 0.
+Otherwise,
+.Dv EOF
+is returned and the global variable
+.Va errno
+is set to indicate the error.
+.Pp
+The
+.Fn fdclose
+function return the file descriptor if successfull.
 Otherwise,
 .Dv EOF

One of English's arcane rules is that most verbs append an 's' when used with 
singular subjects, so "function returns" shoud be used instead of "function 
return", etc.  I do think for this section it would be good to combine the
descriptions of fclose() and fdclose() when possible, so perhaps something
like:

  "The fcloseall() function returns no value.

   Upon successful completion, fclose() returns 0 and fdclose() returns the
   file descriptor of the underlying file.  Otherwise, EOF is returned and
   the global variable errno is set to indicate the error.  In either case
   no further access to the stream is possible."

This allows "in either case" to still read correctly and makes it clear it
applies to both fclose() and fdclose().

 .Sh ERRORS
+.Bl -tag -width Er
+.It Bq Er EOPNOTSUPP
 The
+.Fa _close
+method in
+.Fa stream
+argument to
+.Fn fdclose ,
+was not default.
+.It Bq Er EBADF
+The
+.Fa stream
+argument to
+.Fn fdclose ,
+does not contains valid file descriptor.
+.El
+.Pp
+The
 .Fn fclose
-function
+and
+.Fn fdclose
+functions
 may also fail and set
 .Va errno

For the errors section, the first error list needs some sort of introductory
text.  Also, this shouldn't claim that fdclose() can return an errno value for 
close(2).

"ERRORS

   The fdclose() function may will fail if:

   [EOPNOTSUPP]   The stream to close uses a non-default close method.

   [EBADF]        The stream is not backed by a valid file descriptor.

   The fclose() and fdclose() functions may also fail and set errno for any of
   the errors specified for fflush(3).

   The fclose() functino may also fail and set errno for any of the errors
   specified for close(2)."

_at__at_ -84,7 +130,9 _at__at_
 .Sh NOTES
 The
 .Fn fclose
-function
+and
+.Fn fdclose
+functions
 does not handle NULL arguments; they will result in a segmentation
 violation.
 This is intentional - it makes it easier to make sure programs written

"do not handle".

-- 
John Baldwin