Re: filedesc(9) rewrite, please review

["Rocky Hotas" <rockyhotas%post.com@localhost>]

Hello!

> Sent: Monday, October 15, 2018 at 5:05 PM
> From: coypu%sdf.org@localhost
> To: netbsd-docs%netbsd.org@localhost
> Subject: filedesc(9) rewrite, please review

[...]

> Please review.

Be aware that I do not know the code, so this is definitely from a user-only
perspective. I hope it can be helpful anyway, as it does not always overlap
to the already received Robert Elz review.

Rocky


FILEDESC(9) review

Functions in `NAME' section seem to be listed in alphabetical order. Why not
not to keep the same criterion for the `SYNOPSIS' section, and inside the
two lists inside `FUNCTIONS'?

> The kernel maintains a descriptor table for each process which is used to

For each process, the kernel maintains a descriptor table, which is used to

> The file descriptor table maintains the following information:

The table maintains the following information:

>      the number of descriptors allocated in the file descriptor table;
>      approximate next free descriptor;
>      a reference count on the file descriptor table; and
>      an array of open file entries.

    - the number of descriptors allocated in the file descriptor table;
    - approximate next free descriptor;
    - a reference count on the file descriptor table; and
    - an array of open file entries.
(or whatever list notation the formatting language allows you to use)

> It is the responsibility of the file descriptor operations
> to expand the available number of entries if more are required.

The file descriptor operations are responsible to expand the available
number of entries if more are required.

> information necessary

information needed

> to access the underlying object and to maintain common information

I don't understand.

> purpose specific

Maybe purpose-specific is better?

> fd_alloc(p, want, *result)
> 	      Create a new open file entry and allocate a file descriptor
> 	      for the process p.  

Create a new open file entry in the file descriptor table and allocate a
file descriptor for the process p.

>             This operation is performed by invoking
> 	      fd_alloc() to allocate the new file descriptor.

Isn't this a repetition, or a trivial statement?!

> The fd_alloc() function returns zero on success, otherwise an
> appropriate error is returned.

The fd_alloc() function returns zero on success, or an appropriate error
value otherwise.

> Get the file entry for file descriptor fd The file entry is

Get the file entry for file descriptor fd. The file entry is

> If there is already an open file, close it (See dup2(2)).

(IIUC)
If it is related to an already open file, close it and then use the file
descriptor.

>      fd_dupopen(old, *newp, error)
>	      Duplicate file descriptor specified in old for the current
>             lwp.

IIUC, by LWP you are referring to Light-Weight Processes. Shouldn't be
necessary to specify what an LWP is (that is: give more context)?

>    The following functions operate on the file descriptor table for a
>    process.

Above, you were writing:

>     The following functions are high-level interface routines to access
>     the file descriptor table for a process and its file entries.

What is the difference between them?

>     fd_alloc(p, want, *result)
>	      Allocate a file descriptor want for process p.  The resultant
>	      file descriptor is returned in *result.  The fd_alloc()
>             function returns zero on success, otherwise an appropriate
>	      error is returned.

Is this the same function as the one above?! And which of the two
descriptions is the correct one?

>             It always returns the in-kernel errno value
>	      EMOVEFD, which is meant to be returned from the device open
>	      routine.	This special return value is interpreted by the
> 	      caller of the device open routine.

It always returns the in-kernel errno EMOVEFD. This special value is meant
to be returned from the device open routine; it is interpreted by the caller
of the device open routine.

>  fd_checkstd(l)
>	      Check the standard file descriptors 0, 1, and 2 and ensure
>	      they are referencing valid file descriptors. 

Maybe are you meaning `ensure they are referencing valid files', or `ensure
they are valid'?

>             This operation is necessary as these
>	      file descriptors are given implicit significance in the
>             Standard C Library and it is unsafe for setuid(2) and
>             setgid(2) processes to be started with these file descriptors
> 	      closed.

I don't understand.

> A failed operation will return a non-zero return value.

A failed operation will return a non-zero value.