svn commit: r336746 - in head/lib: libc/gen libutil

[Ian Lepore]

On Fri, 2018-07-27 at 18:03 +0300, Konstantin Belousov wrote:
> On Thu, Jul 26, 2018 at 06:34:38PM +0000, Ian Lepore wrote:
> > 
> > Author: ian
> > Date: Thu Jul 26 18:34:38 2018
> > New Revision: 336746
> > URL: https://svnweb.freebsd.org/changeset/base/336746
> > 
> > Log:
> >   Make pw_scan(3) more compatible with getpwent(3) et. al. when processing
> >   data from /etc/passwd rather than /etc/master.passwd.
> >   
> >   The libc getpwent(3) and related functions automatically read master.passwd
> >   when run by root, or passwd when run by a non-root user.  When run by non-
> >   root, getpwent() copes with the missing data by setting the corresponding
> >   fields in the passwd struct to known values (zeroes for numbers, or a
> >   pointer to an empty string for literals).  When libutil's pw_scan(3) was
> >   used to parse a line without the root-accessible data, it was leaving
> >   garbage in the corresponding fields.
> >   
> >   These changes rename the static pw_init() function used by getpwent() and
> >   friends to __pw_initpwd(), and move it into pw_scan.c so that common init
> >   code can be shared between libc and libutil.  pw_scan(3) now calls
> >   __pw_initpwd() before __pw_scan(), just like the getpwent() family does, so
> >   that reading an arbitrary passwd file in either format and parsing it with
> >   pw_scan(3) returns the same results as getpwent(3) would.
> >   
> >   This also adds a new pw_initpwd(3) function to libutil, so that code which
> >   creates passwd structs from scratch in some manner that doesn't involve
> >   pw_scan() can initialize the struct to the values expected by lots of
> >   existing code, which doesn't expect to encounter NULL pointers or garbage
> >   values in some fields.
> > 
> If my reading is right, you just made libutil depend on the internal
> libc interfaces. Formal consequence is that libutil.so version must
> be bumped each time the used interface is changed (and it is allowed
> to change). I think that your change actually requires the bump of
> libutil.so.N version already.
> 
> Also, libutil.so.N should be moved from the libutil pkgbase package to
> the clibs package, I am not sure about this.
> 
> At the higher level, I very much dislike this change. FBSDprivate_1.0
> namespace is for symbols providing the internal interfaces for the
> C runtime implementation in the FreeBSD. This is mostly a knot of
> inter-dependencies between rtld, libc and libthr. libutil arguably
> should not participate.
> 
> If you want for libc to provide a functionality outside the C runtime,
> please make the sustainable interface, which ABI can be maintained, and
> export the symbols in the normal namespace, with the usual stability
> guarantees.

There was already a function, __pw_scan(), in file pw_scan.c, which was
called from both libutil and libc implementations. I added a new
function __pw_initpwd() into the pw_scan.c file. That function is
called from all the same places that __pw_scan() is called from. So as
near as I can tell, I haven't changed the structure of anything or
created any new linkages between the libraries that didn't exist
already.

I will admit I don't understand the FBSDprivate_1.0 stuff at all, and
there appears to be no documentation or guidance on how to work with
it. Since __pw_scan was in the private list, and I was adding a new
function that is like it in every way, I reasoned that the new function
should be in the list too. It's actually not clear to me that either of
the functions should be in that list, but like I said... no published
info about it that I could find.

I also noticed that chpass(1) and pwd_mkdb(8)_both directly compile in
their own copy of the pw_scan.c source using .PATH in their makefiles.
I wonder if doing that as the way of sharing the code between libc and
libutil would be a better thing to do? (And presumably that would
remove the need to have entries in the FBSDprivate_1.0 list?)

-- Ian