On Tue, 28.10.14 09:53, Martin Pitt (martin.p...@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 _______________________________________________ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel