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

[Ian Lepore]

On Fri, 2018-07-27 at 19:44 +0300, Konstantin Belousov wrote:
> On Fri, Jul 27, 2018 at 10:07:11AM -0600, Ian Lepore wrote:
> > 
> > I believe pw_scan() was originally a static function in libc used by
> > getpwent() and related functions. When libutil grew some pw utilties,
> > it looks like the static pw_scan was renamed to __pw_scan and added to
> > the private list, so that it could be shared with libutil (and so I
> > copied that process with pw_init(), but I had to rename it to
> > pw_initpwd because libutil already has a pw_init()).
> > 
> > I'm not sure how to make the functions more generic, pw_scan() parses a
> > line of text in passwd(5) format, optionally warns about errors, and
> > fills in a struct passwd with what it finds. pw_initpwd() inits a
> > struct passwd to (well-)known default values. But libutil pw_scan()
> > allocates the struct and requires the caller to free it, and the
> > internal __pw_scan() fills in a struct passed in to it and returns an
> > int, so there's no way to re-unify the functions under a single name.
> I did not asked to make it more generic.  I mean, if the function is
> exported, its interface should be useful enough for generic consumers.
> If the current interface is fine, so be it, but see below.
> 
> > 
> > 
> > It would be strange to me to have one or two of the pw_xxxx() family of
> > functions in libc and the rest of them in libutil. Libutil seems like a
> > fine place for password/group file utilities that go beyond the posix
> > functions. It's just an implementation detail that we'd prefer to share
> > the source code for a small bit of common functionality around parsing
> > lines of passwd file data.
> Would it be a solution to stop exporting these functions at all, and
> just sompile them twice, once in libc, and second time in libutil ?
> libutil would add a source file from libc into it SRCS.

Yes, this is exactly what I think we should do after your description
of how FBSDprivate_1.0 exists specifically to solve some interactions
between rtld, libc, and libthr.

This is what I meant when I said in one of the earlier emails:

> 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?)

So if we do that, are there any special considerations about removing
the __pw_scan entry (and now __pw_initpwd as well) from the private
list? Or can they just be deleted without needing to do anything else?
Does anything need to be done to make the __pw_scan symbol not directly
visible for linking to resolve external references?

-- Ian