From 52fffd30c29b98fe9609a2b854a82f6cd3afdfa5 Mon Sep 17 00:00:00 2001 From: Otavio Pontes Date: Fri, 25 Oct 2019 14:08:25 -0700 Subject: [PATCH] sys: Add a wrapper to basename Basename is a very tricky function because there are 2 different implementations available, POXIS and GNU. They can have different outputs and GNU is readonly and POSIX is read-write. For more information take a look at the GNU basename manual. In most areas of swupd it was expected to have the GNU basename used, but when dirname was needed library libgen.h was included and the POSIX basename is used instead. As incorrect usages of basename can cause memory problems a wrapper was created to make sure we are always using the GNU basename, unless specified. in verify and staging there are calls to the POSIX version of basename and it looks like this expected from the code. So it wasn't changed for now. Signed-off-by: Otavio Pontes --- src/alias.c | 2 +- src/autoupdate.c | 1 - src/bundle.c | 1 - src/bundle_add.c | 1 - src/bundle_list.c | 12 +++++------- src/bundle_remove.c | 1 - src/delta.c | 1 - src/hashdump.c | 2 +- src/lib/sys.c | 18 +++++++++++++++--- src/lib/sys.h | 12 ++++++++++++ src/main.c | 8 ++++---- src/telemetry.c | 5 ++--- src/update.c | 1 - 13 files changed, 40 insertions(+), 25 deletions(-) diff --git a/src/alias.c b/src/alias.c index f91b940c..db765080 100644 --- a/src/alias.c +++ b/src/alias.c @@ -173,7 +173,7 @@ struct list *get_alias_definitions(void) iters = system_alias_files; iteru = user_alias_files; while (iters && iteru) { - int pivot = strcmp(basename(iteru->data), basename(iters->data)); + int pivot = strcmp(sys_basename(iteru->data), sys_basename(iters->data)); if (pivot == 0) { if (iters == system_alias_files) { system_alias_files = iters->next; diff --git a/src/autoupdate.c b/src/autoupdate.c index 07eb202b..aebbc210 100644 --- a/src/autoupdate.c +++ b/src/autoupdate.c @@ -22,7 +22,6 @@ #define _GNU_SOURCE #include #include -#include #include #include #include diff --git a/src/bundle.c b/src/bundle.c index e9eb2bd4..43e56496 100644 --- a/src/bundle.c +++ b/src/bundle.c @@ -23,7 +23,6 @@ #define _GNU_SOURCE #include -#include #include #include #include diff --git a/src/bundle_add.c b/src/bundle_add.c index 7dc796b5..cd7e739d 100644 --- a/src/bundle_add.c +++ b/src/bundle_add.c @@ -26,7 +26,6 @@ #include #include #include -#include #include #include #include diff --git a/src/bundle_list.c b/src/bundle_list.c index e0e3bc4e..2e75c999 100644 --- a/src/bundle_list.c +++ b/src/bundle_list.c @@ -23,7 +23,6 @@ #define _GNU_SOURCE #include #include -#include #include #include "config.h" @@ -156,20 +155,19 @@ skip_mom: while (item) { if (MoM) { - bundle_manifest = mom_search_bundle(MoM, basename((char *)item->data)); + bundle_manifest = mom_search_bundle(MoM, sys_basename((char *)item->data)); } if (bundle_manifest) { name = get_printable_bundle_name(bundle_manifest->filename, bundle_manifest->is_experimental); + print("%s\n", name); + free(name); } else { - string_or_die(&name, basename((char *)item->data)); + print("%s\n", sys_basename((char *)item->data)); } - print("%s\n", name); - free_string(&name); - free(item->data); item = item->next; } - list_free_list(bundles); + list_free_list_and_data(bundles, free); free_string(&path); manifest_free(MoM); diff --git a/src/bundle_remove.c b/src/bundle_remove.c index 37d947de..576feb4e 100644 --- a/src/bundle_remove.c +++ b/src/bundle_remove.c @@ -25,7 +25,6 @@ #define _GNU_SOURCE #include #include -#include #include #include #include diff --git a/src/delta.c b/src/delta.c index a51a1582..903b5652 100644 --- a/src/delta.c +++ b/src/delta.c @@ -25,7 +25,6 @@ #include #include #include -#include #include #include #include diff --git a/src/hashdump.c b/src/hashdump.c index 90133639..a28ad4ef 100644 --- a/src/hashdump.c +++ b/src/hashdump.c @@ -47,7 +47,7 @@ static struct option opts[] = { static void usage(const char *name) { print("Usage:\n"); - print(" swupd %s [OPTION...] filename\n\n", basename((char *)name)); + print(" swupd %s [OPTION...] filename\n\n", sys_basename(name)); print("Help Options:\n"); print(" -h, --help Show help options\n\n"); print("Application Options:\n"); diff --git a/src/lib/sys.c b/src/lib/sys.c index b664a0e6..839f3178 100644 --- a/src/lib/sys.c +++ b/src/lib/sys.c @@ -17,22 +17,25 @@ * */ +// Make sure we are getting the gnu version of basename #define _GNU_SOURCE -#include "sys.h" +#include +#undef basename +#include + #include "list.h" #include "log.h" #include "macros.h" #include "memory.h" #include "strings.h" +#include "sys.h" #include #include #include -#include #include #include #include -#include #include #include #include @@ -354,6 +357,15 @@ char *sys_dirname(const char *path) return dir; } +char *sys_basename(const char *path) +{ + if (!path) { + return NULL; + } + + return basename(path); +} + char *sys_path_join(const char *prefix, const char *path) { size_t len = 0; diff --git a/src/lib/sys.h b/src/lib/sys.h index 5899e49f..ced3f310 100644 --- a/src/lib/sys.h +++ b/src/lib/sys.h @@ -163,6 +163,18 @@ bool systemd_in_container(void); */ char *sys_dirname(const char *path); +/** + * @brief Safe GNU implementation of basename + * + * Make sure the GNU implementation of basename will be used and not the + * posix one. The GNU implementation is read-only and won't change the value + * of path. For more information consult GNU basename manual. + * + * A pointer for a portion of the path string will be returned and shoudn't be + * freed. If you need to keep it you need to strdup it. + */ +char *sys_basename(const char *path); + /** * @brief Join 2 paths using the default path separator. * diff --git a/src/main.c b/src/main.c index 767b3a7e..e15bdc59 100644 --- a/src/main.c +++ b/src/main.c @@ -17,7 +17,7 @@ * */ -#define _GNU_SOURCE // for basename() +#define _GNU_SOURCE #include #include #include @@ -81,8 +81,8 @@ static const struct option prog_opts[] = { static void print_help(const char *name) { print("Usage:\n"); - print(" %s [OPTION...]\n", basename((char *)name)); - print(" or %s [OPTION...] SUBCOMMAND [OPTION...]\n\n", basename((char *)name)); + print(" %s [OPTION...]\n", sys_basename(name)); + print(" or %s [OPTION...] SUBCOMMAND [OPTION...]\n\n", sys_basename(name)); print("Help Options:\n"); print(" -h, --help Show help options\n"); print(" -v, --version Output version information and exit\n\n"); @@ -95,7 +95,7 @@ static void print_help(const char *name) entry++; } print("\n"); - print("To view subcommand options, run `%s SUBCOMMAND --help'\n", basename((char *)name)); + print("To view subcommand options, run `%s SUBCOMMAND --help'\n", sys_basename(name)); } /* this function prints the copyright message for the --version command */ diff --git a/src/telemetry.c b/src/telemetry.c index 86f8c1f2..c0fc584d 100644 --- a/src/telemetry.c +++ b/src/telemetry.c @@ -23,7 +23,6 @@ #define _GNU_SOURCE #include -#include #include #include #include @@ -59,8 +58,8 @@ void telemetry(telem_prio_t level, const char *class, const char *fmt, ...) close(fd); - filename_n = basename(filename); - if (!filename_n) { + filename_n = sys_basename(filename); + if (!filename_n || !filename_n[0]) { free_string(&filename); goto error; } diff --git a/src/update.c b/src/update.c index 3adbe697..5276d481 100644 --- a/src/update.c +++ b/src/update.c @@ -25,7 +25,6 @@ #include #include #include -#include #include #include #include