Re: [PATCH] New sol10priv module

[Jim Meyering]

David Bartley wrote:
> The following patches for coreutils and gnulib add a new Solaris 10
> privilege module as previously suggested [1].

Hi David,

Thanks for following through.
Are you up to the task of making a few more changes?

============================================================
- naming: the module and function names should be OS-agnostic,
in case some day we extend it to other than Solaris 10.
For example, I'll bet they already work on opensolaris.
Maybe something like priv-set.[ch] for module/file names
and priv_set_* for function names.
By the say, with git, you can perform the requested renaming
trivially via this trick:
Take your git format-patch output (what you mailed to this list),
and perform the global substitutions:

    perl -pe 's/sol10priv_/priv_set_/g;s/sol10priv.([chm])/priv-set.$1/g;'
      -e s/sol10priv/priv_set/g FILE > FILE-new

Then apply that patch on a new branch like this:

    git co master
    git co -b new
    git am FILE-new

and that part is done.

============================================================
- Neither rm.c nor mv.c includes your new .h file, yet they must,
in order to get the new declaration.

============================================================
- I want to be able to use a function like *priv_remove without in-function
#ifdefs for every use.  One way to do that is to put all of the ifdefs
in priv-set.h.  E.g.,

#if ...
  declare the functions
#else
  define static inline no-op functions by the same names
#endif

============================================================
Finally, since we'd like to be OS-agnostic,
we can't include Solaris-10-specific symbols
like PRIV_SYS_LINKDIR in the public API:

    > +  sol10priv_remove (PRIV_SYS_LINKDIR);

Simplest would be to add a more-specialized inline function,
priv_remove_linkdir, that would do the job (or be a no-op).

The alternative is to define a new gnulib-specific enum symbol,
and use this in the client code:

         priv_remove (GL_PRIV_SYS_LINKDIR);

where the function would then map that enum to the OS-specific value.

============================================================
Then, each client should not have to include this.
  #if HAVE_GETPPRIV
  # include <priv.h>
  #endif
Rather, just include that from your new .c file.

============================================================

Please ensure that the result passes "make check"
after building with /configure --enable-gcc-warnings.