Re: [systemd-devel] [RFC 21/25] make sure basename that doesn't alter it's argument

2014-10-10 Thread Lennart Poettering 
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));
  
  return u-unit_file_state;
  }
 diff --git a/src/delta/delta.c b/src/delta/delta.c
 index 91f8592..cb049e8 100644
 ---

[systemd-devel] [RFC 21/25] make sure basename that doesn't alter it's argument

2014-09-18 Thread Emil Renner Berthing 
---
 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(p),

Re: [systemd-devel] [RFC 21/25] make sure basename that doesn't alter it's argument

2014-09-18 Thread Tom Gundersen 
This needs a bit more explanation. Why this change?

Cheers,

Tom

On Thu, Sep 18, 2014 at 3:24 PM, Emil Renner Berthing syst...@esmil.dk 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
  return -ENOMEM;

  log_debug(Adding to drops: %s %s %s %s %s,
 -  unit,

Re: [systemd-devel] [RFC 21/25] make sure basename that doesn't alter it's argument

2014-09-18 Thread Cristian Rodríguez 
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