On Sat, Apr 12, 2014 at 12:07:04PM -0400, Mike Gilbert wrote: > Modifies find_binary() to accept NULL in the second argument. > > fsck.type lookup logic moved to new fsck_exists() function, with a test. > --- > src/fsck/fsck.c | 9 ++++----- > src/shared/generator.c | 11 ++++------- > src/shared/path-util.c | 11 ++++++++--- > src/shared/util.c | 8 ++++++++ > src/shared/util.h | 2 ++ > src/test/test-util.c | 11 +++++++++++ > 6 files changed, 37 insertions(+), 15 deletions(-) > > diff --git a/src/fsck/fsck.c b/src/fsck/fsck.c > index 18f2aca..520b1a6 100644 > --- a/src/fsck/fsck.c > +++ b/src/fsck/fsck.c > @@ -285,14 +285,13 @@ int main(int argc, char *argv[]) { > > type = udev_device_get_property_value(udev_device, "ID_FS_TYPE"); > if (type) { > - const char *checker = strappenda("/sbin/fsck.", type); > - r = access(checker, X_OK); > + r = fsck_exists(type); > if (r < 0) { > - if (errno == ENOENT) { > - log_info("%s doesn't exist, not checking > file system.", checker); > + if (r == -ENOENT) { > + log_info("fsck.%s doesn't exist, not > checking file system.", type); > return EXIT_SUCCESS; > } else > - log_warning("%s cannot be used: %m", > checker); > + log_warning("fsck.%s cannot be used: %s", > type, strerror(-r)); > } > } > > diff --git a/src/shared/generator.c b/src/shared/generator.c > index 6110303..86d30e7 100644 > --- a/src/shared/generator.c > +++ b/src/shared/generator.c > @@ -19,6 +19,7 @@ > along with systemd; If not, see <http://www.gnu.org/licenses/>. > ***/ > > +#include <string.h> > #include <unistd.h> > > #include "util.h" > @@ -45,16 +46,12 @@ int generator_write_fsck_deps( > } > > if (!isempty(fstype) && !streq(fstype, "auto")) { > - const char *checker; > int r; > - > - checker = strappenda("/sbin/fsck.", fstype); > - r = access(checker, X_OK); > + r = fsck_exists(fstype); > if (r < 0) { > - log_warning("Checking was requested for %s, but %s > cannot be used: %m", what, checker); > - > + log_warning("Checking was requested for %s, but > fsck.%s cannot be used: %s", what, what, strerror(-r));
The second arg should probably be fstype not what. > /* treat missing check as essentially OK */ > - return errno == ENOENT ? 0 : -errno; > + return r == -ENOENT ? 0 : r; > } > } > > diff --git a/src/shared/path-util.c b/src/shared/path-util.c > index bdc54a9..a530dbe 100644 > --- a/src/shared/path-util.c > +++ b/src/shared/path-util.c > @@ -425,7 +425,6 @@ int path_is_os_tree(const char *path) { > > int find_binary(const char *name, char **filename) { > assert(name); > - assert(filename); > > if (strchr(name, '/')) { > char *p; > @@ -437,7 +436,10 @@ int find_binary(const char *name, char **filename) { > if (!p) > return -ENOMEM; > > - *filename = p; > + if (filename) > + *filename = p; > + else > + free(p); Allocating a string just to free it in the next line doesn't make sense. > return 0; > } else { > const char *path; > @@ -464,7 +466,10 @@ int find_binary(const char *name, char **filename) { > } > > path_kill_slashes(p); > - *filename = p; > + if (filename) > + *filename = p; > + else > + free(p); Likewise, no need to mangle the string just to free it. > > return 0; > } > diff --git a/src/shared/util.c b/src/shared/util.c > index ffe6624..4cdb561 100644 > --- a/src/shared/util.c > +++ b/src/shared/util.c > @@ -6391,3 +6391,11 @@ void hexdump(FILE *f, const void *p, size_t s) { > s -= 16; > } > } > + > +int fsck_exists(const char *fstype) > +{ Brace should go on the end of previous line. > + const char *checker; > + > + checker = strappenda("fsck.", fstype); > + return find_binary(checker, NULL); > +} > diff --git a/src/shared/util.h b/src/shared/util.h > index 90464c9..45a6f26 100644 > --- a/src/shared/util.h > +++ b/src/shared/util.h > @@ -922,3 +922,5 @@ uint64_t physical_memory(void); > char* mount_test_option(const char *haystack, const char *needle); > > void hexdump(FILE *f, const void *p, size_t s); > + > +int fsck_exists(const char *fstype); > diff --git a/src/test/test-util.c b/src/test/test-util.c > index 93929cd..148e54c 100644 > --- a/src/test/test-util.c > +++ b/src/test/test-util.c > @@ -675,6 +675,16 @@ static void test_foreach_string(void) { > assert_se(streq(x, "zzz")); > } > > +static void test_fsck_exists(void) { > + /* Ensure we use a sane default for PATH. */ > + unsetenv("PATH"); > + > + /* fsck.minix is provided by util-linux and will probably exist. */ > + assert(fsck_exists("minix") == 0); > + > + assert(fsck_exists("AbCdE") == -ENOENT); > +} assert_se(). > + > int main(int argc, char *argv[]) { > log_parse_environment(); > log_open(); > @@ -719,6 +729,7 @@ int main(int argc, char *argv[]) { > test_hexdump(); > test_log2i(); > test_foreach_string(); > + test_fsck_exists(); > > return 0; > } Zbyszek _______________________________________________ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel