[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH] New sol10priv module

From: Bruno Haible
Subject: Re: [PATCH] New sol10priv module
Date: Wed, 29 Apr 2009 12:26:40 +0200
User-agent: KMail/1.9.9

Hello David,

Thank you for your module, and your patience regarding the paperwork.

David Bartley wrote:
> The following patches for coreutils and gnulib add a new Solaris 10
> privilege module

Five small remarks about the module:

1) In the module description:

> +lib/sol10priv.h
> +lib/sol10priv.c
> +lib_SOURCES += sol10priv-remove.c sol10priv-restore.c

This will not work. You provide the file sol10priv.c and tell the compiler
to compile two other, different files?

You can try to compile your module through the command
  $ ./gnulib-tool --test sol10priv

2) The specification of the functions is ambiguous.

> /* Try to restore priv if it was previously removed from the effective set.  
> */

A specification should allow to write code that invokes a function, without
looking at its implementation. This implies that it should answer the questions:
  - What does the function do? (You have this already answered.)
  - What is the return value?
  - In which ways can the function fail? Does it set errno when it fails?

> +#include <config.h>
> +

When a compilation unit is empty after preprocessing, some compilers warn.
There are two possible workarounds:
  - Add some dummy typedef, as in gnulib/lib/canonicalize-lgpl.c,
  - Put the "#if HAVE_GETPPRIV" after #include "sol10priv.h".

4) Would it be possible to add a unit test? Maybe using the 'ppriv'
   command? See [1][2].

> +2009-04-27  David Bartley  <address@hidden>
> +        New module 'sol10priv'.

It's customary to put a newline after the author line in a ChangeLog entry.


[1] http://docs.sun.com/app/docs/doc/816-5175/privileges-5?a=view
[2] http://docs.sun.com/app/docs/doc/816-5165/ppriv-1?a=view

reply via email to

[Prev in Thread] Current Thread [Next in Thread]