[systemd-devel] [PATCH v5] udev hwdb: Support shipping pre-compiled database in system images

[Lennart Poettering]

On Tue, 28.10.14 09:53, Martin Pitt (martin.pitt at ubuntu.com) wrote:

> Hello Lennart,
> 
> Lennart Poettering [2014-10-27 16:09 +0100]:
> > > +static const char hwdb_bin_paths[] =
> > > +    "/etc/udev/hwdb.bin\0"
> > > +    UDEVLIBEXECDIR "/hwdb.bin\0";
> > > +
> > > +
> > > +static int open_hwdb_bin(const char **path, FILE** f) {
> > > +        const char* p;
> > > +
> > > +        NULSTR_FOREACH(p, hwdb_bin_paths) {
> > > +                *path = p;
> > 
> > Please do not write functions that clobber passed-in arguments on
> > failure. Please store any result in a temporary variable first, and
> > clobber the passed-in variables only on success.
> > 
> > > +                *f = fopen(p, "re");
> > 
> > same here.
> 
> Done for the FILE*, but I kept the behaviour for the path as the
> caller uses the path in the error message on failure. I added a
> comment to clarify this.

I'd prefer if you'd move the log message for the error into
open_hwdb_bin() then, so that it is not the caller, but the callee
which prints the error message in this case.

Otherwise looks good.

Lennart

-- 
Lennart Poettering, Red Hat