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

[Konstantin Belousov]

On Fri, Jul 27, 2018 at 09:35:41AM -0600, Ian Lepore wrote:
> 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?)

I suspect that the better way is to export the used functions
in the FBSD_1.5 namespace.  Might be use the opportunity to rename
them, in particular, remove the leading double underscore.  Might
be, reconsider the interfaces to make them more generic.

Do the consumers depend on the specific layout of some structure ?