Re: [systemd-devel] [RFC 21/25] make sure basename that doesn't alter it's argument
On Thu, 18.09.14 15:24, Emil Renner Berthing (syst...@esmil.dk) wrote: What's the rationale here? The GNU version of basename() appears a lot more useful than the POSIX. I am find with switching to the POSIX version of libc calls if both have more or less equivalent functionality or they are used very seldom only, but in this case the GNU logic seems so much more useful and we use the call all the time. > --- > src/core/execute.c | 6 +++--- > src/core/load-fragment.c| 2 +- > src/core/manager.c | 2 +- > src/core/unit.c | 4 ++-- > src/delta/delta.c | 14 +++--- > src/journal/journalctl.c| 2 +- > src/locale/localectl.c | 2 +- > src/login/logind-inhibit.c | 2 +- > src/login/logind-seat.c | 2 +- > src/login/logind-session.c | 2 +- > src/nspawn/nspawn.c | 2 +- > src/shared/cgroup-show.c| 4 ++-- > src/shared/conf-files.c | 4 ++-- > src/shared/install.c| 14 +++--- > src/shared/path-util.c | 8 > src/shared/path-util.h | 5 + > src/shared/util.c | 4 ++-- > src/shared/utmp-wtmp.c | 2 +- > src/systemctl/systemctl.c | 12 ++-- > src/sysv-generator/sysv-generator.c | 4 ++-- > src/test/test-install.c | 18 +- > src/test/test-path-util.c | 8 > 22 files changed, 68 insertions(+), 55 deletions(-) > > diff --git a/src/core/execute.c b/src/core/execute.c > index e73eb8e..77724ce 100644 > --- a/src/core/execute.c > +++ b/src/core/execute.c > @@ -912,7 +912,7 @@ static void rename_process_from_path(const char *path) { > /* This resulting string must fit in 10 chars (i.e. the length > * of "/sbin/init") to look pretty in /bin/ps */ > > -p = basename(path); > +p = path_basename(path); > if (isempty(p)) { > rename_process("(...)"); > return; > @@ -1331,13 +1331,13 @@ static int exec_child(ExecCommand *command, > return err; > } > > -err = setup_output(context, STDOUT_FILENO, socket_fd, > basename(command->path), params->unit_id, params->apply_tty_stdin); > +err = setup_output(context, STDOUT_FILENO, socket_fd, > path_basename(command->path), params->unit_id, params->apply_tty_stdin); > if (err < 0) { > *error = EXIT_STDOUT; > return err; > } > > -err = setup_output(context, STDERR_FILENO, socket_fd, > basename(command->path), params->unit_id, params->apply_tty_stdin); > +err = setup_output(context, STDERR_FILENO, socket_fd, > path_basename(command->path), params->unit_id, params->apply_tty_stdin); > if (err < 0) { > *error = EXIT_STDERR; > return err; > diff --git a/src/core/load-fragment.c b/src/core/load-fragment.c > index 0620882..61db112 100644 > --- a/src/core/load-fragment.c > +++ b/src/core/load-fragment.c > @@ -3322,7 +3322,7 @@ static int open_follow(char **filename, FILE **_f, Set > *names, char **_final) { > /* Add the file name we are currently looking at to > * the names of this unit, but only if it is a valid > * unit name. */ > -name = basename(*filename); > +name = path_basename(*filename); > > if (unit_name_is_valid(name, TEMPLATE_VALID)) { > > diff --git a/src/core/manager.c b/src/core/manager.c > index 88d660d..22a9c89 100644 > --- a/src/core/manager.c > +++ b/src/core/manager.c > @@ -1217,7 +1217,7 @@ int manager_load_unit_prepare( > return sd_bus_error_setf(e, SD_BUS_ERROR_INVALID_ARGS, "Path > %s is not absolute.", path); > > if (!name) > -name = basename(path); > +name = path_basename(path); > > t = unit_name_to_type(name); > > diff --git a/src/core/unit.c b/src/core/unit.c > index def5c36..c14859e 100644 > --- a/src/core/unit.c > +++ b/src/core/unit.c > @@ -2170,7 +2170,7 @@ static const char *resolve_template(Unit *u, const char > *name, const char*path, > assert(p); > > if (!name) > -name = basename(path); > +name = path_basename(path); > > if (!unit_name_is_template(name)) { > *p = NULL; > @@ -2941,7 +2941,7 @@ UnitFileState unit_get_unit_file_state(Unit *u) { > if (u->unit_file_state < 0 && u->fragment_path) > u->unit_file_state = unit_file_get_state( > u->manager->running_as == SYSTEMD_SYSTEM ? > UNIT_FILE_SYSTEM : UNIT_FILE_USER, > -NULL, basename(u->fragment_path)); > +NULL, path_basename(u->fragment_path)
Re: [systemd-devel] [RFC 21/25] make sure basename that doesn't alter it's argument
El 18/09/14 a las #4, Emil Renner Berthing escribió: > --- And all of this because the POSIX versions of basename modify the argument .. see the problem ? it is the standard versions that are wrong. what about implementing a gnu_basename in the C library and using that instead ? -- Cristian "I don't know the key to success, but the key to failure is trying to please everybody." ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [RFC 21/25] make sure basename that doesn't alter it's argument
This needs a bit more explanation. Why this change? Cheers, Tom On Thu, Sep 18, 2014 at 3:24 PM, Emil Renner Berthing wrote: > --- > src/core/execute.c | 6 +++--- > src/core/load-fragment.c| 2 +- > src/core/manager.c | 2 +- > src/core/unit.c | 4 ++-- > src/delta/delta.c | 14 +++--- > src/journal/journalctl.c| 2 +- > src/locale/localectl.c | 2 +- > src/login/logind-inhibit.c | 2 +- > src/login/logind-seat.c | 2 +- > src/login/logind-session.c | 2 +- > src/nspawn/nspawn.c | 2 +- > src/shared/cgroup-show.c| 4 ++-- > src/shared/conf-files.c | 4 ++-- > src/shared/install.c| 14 +++--- > src/shared/path-util.c | 8 > src/shared/path-util.h | 5 + > src/shared/util.c | 4 ++-- > src/shared/utmp-wtmp.c | 2 +- > src/systemctl/systemctl.c | 12 ++-- > src/sysv-generator/sysv-generator.c | 4 ++-- > src/test/test-install.c | 18 +- > src/test/test-path-util.c | 8 > 22 files changed, 68 insertions(+), 55 deletions(-) > > diff --git a/src/core/execute.c b/src/core/execute.c > index e73eb8e..77724ce 100644 > --- a/src/core/execute.c > +++ b/src/core/execute.c > @@ -912,7 +912,7 @@ static void rename_process_from_path(const char *path) { > /* This resulting string must fit in 10 chars (i.e. the length > * of "/sbin/init") to look pretty in /bin/ps */ > > -p = basename(path); > +p = path_basename(path); > if (isempty(p)) { > rename_process("(...)"); > return; > @@ -1331,13 +1331,13 @@ static int exec_child(ExecCommand *command, > return err; > } > > -err = setup_output(context, STDOUT_FILENO, socket_fd, > basename(command->path), params->unit_id, params->apply_tty_stdin); > +err = setup_output(context, STDOUT_FILENO, socket_fd, > path_basename(command->path), params->unit_id, params->apply_tty_stdin); > if (err < 0) { > *error = EXIT_STDOUT; > return err; > } > > -err = setup_output(context, STDERR_FILENO, socket_fd, > basename(command->path), params->unit_id, params->apply_tty_stdin); > +err = setup_output(context, STDERR_FILENO, socket_fd, > path_basename(command->path), params->unit_id, params->apply_tty_stdin); > if (err < 0) { > *error = EXIT_STDERR; > return err; > diff --git a/src/core/load-fragment.c b/src/core/load-fragment.c > index 0620882..61db112 100644 > --- a/src/core/load-fragment.c > +++ b/src/core/load-fragment.c > @@ -3322,7 +3322,7 @@ static int open_follow(char **filename, FILE **_f, Set > *names, char **_final) { > /* Add the file name we are currently looking at to > * the names of this unit, but only if it is a valid > * unit name. */ > -name = basename(*filename); > +name = path_basename(*filename); > > if (unit_name_is_valid(name, TEMPLATE_VALID)) { > > diff --git a/src/core/manager.c b/src/core/manager.c > index 88d660d..22a9c89 100644 > --- a/src/core/manager.c > +++ b/src/core/manager.c > @@ -1217,7 +1217,7 @@ int manager_load_unit_prepare( > return sd_bus_error_setf(e, SD_BUS_ERROR_INVALID_ARGS, "Path > %s is not absolute.", path); > > if (!name) > -name = basename(path); > +name = path_basename(path); > > t = unit_name_to_type(name); > > diff --git a/src/core/unit.c b/src/core/unit.c > index def5c36..c14859e 100644 > --- a/src/core/unit.c > +++ b/src/core/unit.c > @@ -2170,7 +2170,7 @@ static const char *resolve_template(Unit *u, const char > *name, const char*path, > assert(p); > > if (!name) > -name = basename(path); > +name = path_basename(path); > > if (!unit_name_is_template(name)) { > *p = NULL; > @@ -2941,7 +2941,7 @@ UnitFileState unit_get_unit_file_state(Unit *u) { > if (u->unit_file_state < 0 && u->fragment_path) > u->unit_file_state = unit_file_get_state( > u->manager->running_as == SYSTEMD_SYSTEM ? > UNIT_FILE_SYSTEM : UNIT_FILE_USER, > -NULL, basename(u->fragment_path)); > +NULL, path_basename(u->fragment_path)); > > return u->unit_file_state; > } > diff --git a/src/delta/delta.c b/src/delta/delta.c > index 91f8592..cb049e8 100644 > --- a/src/delta/delta.c > +++ b/src/delta/delta.c > @@ -282,8 +282,8 @@ static int enumerate_dir_d(Hashmap *top, Hashmap *bottom, > Hashmap *drops, const >
[systemd-devel] [RFC 21/25] make sure basename that doesn't alter it's argument
--- src/core/execute.c | 6 +++--- src/core/load-fragment.c| 2 +- src/core/manager.c | 2 +- src/core/unit.c | 4 ++-- src/delta/delta.c | 14 +++--- src/journal/journalctl.c| 2 +- src/locale/localectl.c | 2 +- src/login/logind-inhibit.c | 2 +- src/login/logind-seat.c | 2 +- src/login/logind-session.c | 2 +- src/nspawn/nspawn.c | 2 +- src/shared/cgroup-show.c| 4 ++-- src/shared/conf-files.c | 4 ++-- src/shared/install.c| 14 +++--- src/shared/path-util.c | 8 src/shared/path-util.h | 5 + src/shared/util.c | 4 ++-- src/shared/utmp-wtmp.c | 2 +- src/systemctl/systemctl.c | 12 ++-- src/sysv-generator/sysv-generator.c | 4 ++-- src/test/test-install.c | 18 +- src/test/test-path-util.c | 8 22 files changed, 68 insertions(+), 55 deletions(-) diff --git a/src/core/execute.c b/src/core/execute.c index e73eb8e..77724ce 100644 --- a/src/core/execute.c +++ b/src/core/execute.c @@ -912,7 +912,7 @@ static void rename_process_from_path(const char *path) { /* This resulting string must fit in 10 chars (i.e. the length * of "/sbin/init") to look pretty in /bin/ps */ -p = basename(path); +p = path_basename(path); if (isempty(p)) { rename_process("(...)"); return; @@ -1331,13 +1331,13 @@ static int exec_child(ExecCommand *command, return err; } -err = setup_output(context, STDOUT_FILENO, socket_fd, basename(command->path), params->unit_id, params->apply_tty_stdin); +err = setup_output(context, STDOUT_FILENO, socket_fd, path_basename(command->path), params->unit_id, params->apply_tty_stdin); if (err < 0) { *error = EXIT_STDOUT; return err; } -err = setup_output(context, STDERR_FILENO, socket_fd, basename(command->path), params->unit_id, params->apply_tty_stdin); +err = setup_output(context, STDERR_FILENO, socket_fd, path_basename(command->path), params->unit_id, params->apply_tty_stdin); if (err < 0) { *error = EXIT_STDERR; return err; diff --git a/src/core/load-fragment.c b/src/core/load-fragment.c index 0620882..61db112 100644 --- a/src/core/load-fragment.c +++ b/src/core/load-fragment.c @@ -3322,7 +3322,7 @@ static int open_follow(char **filename, FILE **_f, Set *names, char **_final) { /* Add the file name we are currently looking at to * the names of this unit, but only if it is a valid * unit name. */ -name = basename(*filename); +name = path_basename(*filename); if (unit_name_is_valid(name, TEMPLATE_VALID)) { diff --git a/src/core/manager.c b/src/core/manager.c index 88d660d..22a9c89 100644 --- a/src/core/manager.c +++ b/src/core/manager.c @@ -1217,7 +1217,7 @@ int manager_load_unit_prepare( return sd_bus_error_setf(e, SD_BUS_ERROR_INVALID_ARGS, "Path %s is not absolute.", path); if (!name) -name = basename(path); +name = path_basename(path); t = unit_name_to_type(name); diff --git a/src/core/unit.c b/src/core/unit.c index def5c36..c14859e 100644 --- a/src/core/unit.c +++ b/src/core/unit.c @@ -2170,7 +2170,7 @@ static const char *resolve_template(Unit *u, const char *name, const char*path, assert(p); if (!name) -name = basename(path); +name = path_basename(path); if (!unit_name_is_template(name)) { *p = NULL; @@ -2941,7 +2941,7 @@ UnitFileState unit_get_unit_file_state(Unit *u) { if (u->unit_file_state < 0 && u->fragment_path) u->unit_file_state = unit_file_get_state( u->manager->running_as == SYSTEMD_SYSTEM ? UNIT_FILE_SYSTEM : UNIT_FILE_USER, -NULL, basename(u->fragment_path)); +NULL, path_basename(u->fragment_path)); return u->unit_file_state; } diff --git a/src/delta/delta.c b/src/delta/delta.c index 91f8592..cb049e8 100644 --- a/src/delta/delta.c +++ b/src/delta/delta.c @@ -282,8 +282,8 @@ static int enumerate_dir_d(Hashmap *top, Hashmap *bottom, Hashmap *drops, const return -ENOMEM; log_debug("Adding to drops: %s %s %s %s %s", - unit, draw_special_char(DRAW_ARROW), basename(p), draw_special_char(DRAW_ARROW), p); -k = hashmap_put(h, basename(p), p); + unit, draw_special_char(DRAW_ARROW), path_basename(