Re: <shadow.h> function: fgetspent_r

[Rich Felker <dalias@libc.org>]

On Wed, Jan 16, 2019 at 03:38:06PM -0600, A. Wilcox wrote:
> On 01/16/19 14:50, Rich Felker wrote:
> > On Wed, Jan 16, 2019 at 01:21:39PM -0600, A. Wilcox wrote:
> >> Hi muslers,
> >>
> >> fgetspent_r[1] is a re-entrant version of fgetspent which stores all
> >> strings in a caller-provided buffer to ensure that the memory is owned
> >> by the caller instead of by the system.
> >>
> >> It is present in Solaris 9[2] and higher, and glibc[3] Linux.  It is
> >> used by AccountsService[4].
> >>
> >> Is it possible to add this API to musl?  I could try to write it, if so.
> >>
> >> Best,
> >> --arw
> >>
> >>
> >> [1]: https://docs.oracle.com/cd/E88353_01/html/E37843/getspent-r-3c.html
> >>
> >> [2]: https://docs.oracle.com/cd/E19683-01/816-5214/6mbcfdl0v/index.html
> >>
> >> [3]: https://linux.die.net/man/3/getspnam_r
> >>
> >> [4]: https://cgit.freedesktop.org/accountsservice/commit/?id=14ca4245
> > 
> > I don't see any good reason why it couldn't be added, but it doesn't
> > look like a direct refactoring of the existing function since it uses
> > the messy char[] buffer idiom like a bunch of other _r functions in
> > this family.
> 
> What do you mean by "messy char[] buffer idiom"?  The buffer that is
> meant to contain the strings is passed to the function (char *buf),
> *not* returned by it.

It's also necessarily used to contain the struct itself, despite C not
really allowing this and the buffer not being properly aligned for it
(requiring realignment which is inherently nonportable and not even
possible in something like a memory-safe implementation). A proper API
would have the caller pass both a pointer to the struct to fill and a
pointer to char[] space for strings.

> > I'm also not sure what should happen if the next entry
> > does not fit in the buffer. Should it discard the rest of the line and
> > move on (not retryable) or attempt to seek back?
> 
> The Solaris docs specify:
> 
> fgetspent_r() [ ... ] return a pointer to a struct spwd if they
> successfully enumerate an entry; otherwise they return NULL, indicating
> the end of the enumeration.
> 
> The reentrant functions getspnam_r(), getspent_r(), and fgetspent_r()
> will return NULL and set errno to ERANGE if the length of the buffer
> supplied by caller is not large enough to store the result.
> 
> 
> The glibc docs specify:
> 
> A pointer to the result (in case of success) or NULL (in case no entry
> was found or an error occurred) is stored in *spbufp.
> 
> The reentrant functions return zero on success. In case of error, an
> error number is returned.
> 
> ERANGE: Supplied buffer is too small.
> 
> 
> 
> It seems like "end of file" and "error" are treated the same.  In any
> case, it would appear to me that it's UB to continue iterating once NULL
> is returned.

Well at least has unspecified behavior; it's not necessarily
undefined.

> I would personally leave the fp where it is (not rewind)
> since all the other *get*ent functions don't rewind on error either.

But should it finish consuming the line it's in the middle of?
Otherwise it could wrongly interpret the remainder of the line as a
complete line if called again. Note that the current version of the
non-_r function has that issue if getline OOM's..

Rich