[systemd-devel] [PATCH] use "Out of memory." consistantly (or with "\n")
Lennart Poettering
lennart at poettering.net
Mon Aug 6 07:39:04 PDT 2012
On Fri, 03.08.12 17:33, shawn (shawnlandden at gmail.com) wrote:
Applied!
Note that we should be carefuly with adding additional erorr messages to
the EXIT_SUCCESS/EXIT_FAILURE bits, since we might end up with two error
messages instead of one, which is something we should try to avoid...
Lennart
> On Thu, 2012-07-26 at 11:51 +0200, Kay Sievers wrote:
> > On Thu, Jul 26, 2012 at 1:17 AM, shawn <shawnlandden at gmail.com> wrote:
> > > On Wed, 2012-07-25 at 18:11 +0200, Lennart Poettering wrote:
> > >> On Wed, 25.07.12 11:24, Kay Sievers (kay at vrfy.org) wrote:
> > >> > On Wed, Jul 25, 2012 at 6:12 AM, Shawn Landden <shawnlandden at gmail.com> wrote:
> > >> > > glibc/glib both use "out of memory" consistantly so maybe we should
> > >> > > consider that instead of this.
> > >> > >
> > >> > > Eliminates one string out of a number of binaries. Also fixes extra newline
> > >> > > in udev/scsi_id
> > >> >
> > >> > Applied.
> > >>
> > >> Hmm, given that we might run into this again, it might make sense to
> > >> define function for this? Maybe something like this in log.h?
> > >>
> > >> static inline void log_oom(void) {
> > >> log_error("Out of memory.");
> > >> return -ENOMEM;
> > >> }
> > >>
> > >> Which we then could use everywhere?
> > >
> > > patch that does that
> >
> > Applied.
>
> another
>
> >From 9f7a1f61c733fa90874d9f813589ece9afbae942 Mon Sep 17 00:00:00 2001
> From: Shawn Landden <shawnlandden at gmail.com>
> Date: Fri, 3 Aug 2012 17:22:09 -0700
> Subject: [PATCH] continue work with error messages, log_oom()
>
> Adds messages for formally silent errors: new "Failed on cmdline argument %s: %s".
>
> Removes some specific error messages for -ENOMEM in mount-setup.c. A few specific
> ones have been left in other binaries.
> ---
> TODO | 2 +-
> src/binfmt/binfmt.c | 4 ++--
> src/cgtop/cgtop.c | 7 ++++++-
> src/core/main.c | 34 ++++++++++++++++++++--------------
> src/core/mount-setup.c | 12 ++++--------
> 5 files changed, 33 insertions(+), 26 deletions(-)
>
> diff --git a/TODO b/TODO
> index ac9bae4..c694ce0 100644
> --- a/TODO
> +++ b/TODO
> @@ -460,7 +460,7 @@ Regularly:
>
> * Use PR_SET_PROCTITLE_AREA if it becomes available in the kernel
>
> -* %m in printf() instead of strerror();
> +* %m in printf() instead of strerror(errno);
>
> * pahole
>
> diff --git a/src/binfmt/binfmt.c b/src/binfmt/binfmt.c
> index 0399ab7..788fd4b 100644
> --- a/src/binfmt/binfmt.c
> +++ b/src/binfmt/binfmt.c
> @@ -40,7 +40,7 @@ static int delete_rule(const char *rule) {
> assert(rule[0]);
>
> if (!(x = strdup(rule)))
> - return -ENOMEM;
> + return log_oom();
>
> e = strchrnul(x+1, x[0]);
> *e = 0;
> @@ -49,7 +49,7 @@ static int delete_rule(const char *rule) {
> free(x);
>
> if (!fn)
> - return -ENOMEM;
> + return log_oom();
>
> r = write_one_line_file(fn, "-1");
> free(fn);
> diff --git a/src/cgtop/cgtop.c b/src/cgtop/cgtop.c
> index a57a468..3756328 100644
> --- a/src/cgtop/cgtop.c
> +++ b/src/cgtop/cgtop.c
> @@ -782,5 +782,10 @@ finish:
> group_hashmap_free(a);
> group_hashmap_free(b);
>
> - return r < 0 ? EXIT_FAILURE : EXIT_SUCCESS;
> + if (r < 0) {
> + log_error("Exiting with failure: %s", strerror(-r));
> + return EXIT_FAILURE;
> + }
> +
> + return EXIT_SUCCESS;
> }
> diff --git a/src/core/main.c b/src/core/main.c
> index 1326b94..3c0f5f9 100644
> --- a/src/core/main.c
> +++ b/src/core/main.c
> @@ -168,12 +168,12 @@ _noreturn_ static void crash(int sig) {
>
> pid = fork();
> if (pid < 0)
> - log_error("Failed to fork off crash shell: %s", strerror(errno));
> + log_error("Failed to fork off crash shell: %m");
> else if (pid == 0) {
> make_console_stdio();
> execl("/bin/sh", "/bin/sh", NULL);
>
> - log_error("execl() failed: %s", strerror(errno));
> + log_error("execl() failed: %m");
> _exit(1);
> }
>
> @@ -350,12 +350,12 @@ static int parse_proc_cmdline_word(const char *word) {
> if (!eq) {
> r = unsetenv(cenv);
> if (r < 0)
> - log_warning("unsetenv failed %s. Ignoring.", strerror(errno));
> + log_warning("unsetenv failed %m. Ignoring.");
> } else {
> *eq = 0;
> r = setenv(cenv, eq + 1, 1);
> if (r < 0)
> - log_warning("setenv failed %s. Ignoring.", strerror(errno));
> + log_warning("setenv failed %m. Ignoring.");
> }
> free(cenv);
>
> @@ -495,14 +495,14 @@ static int config_parse_cpu_affinity2(
> unsigned cpu;
>
> if (!(t = strndup(w, l)))
> - return -ENOMEM;
> + return log_oom();
>
> r = safe_atou(t, &cpu);
> free(t);
>
> if (!c)
> if (!(c = cpu_set_malloc(&ncpus)))
> - return -ENOMEM;
> + return log_oom();
>
> if (r < 0 || cpu >= ncpus) {
> log_error("[%s:%u] Failed to parse CPU affinity: %s", filename, line, rvalue);
> @@ -568,7 +568,7 @@ static int config_parse_join_controllers(
>
> s = strndup(w, length);
> if (!s)
> - return -ENOMEM;
> + return log_oom();
>
> l = strv_split(s, ",");
> free(s);
> @@ -584,7 +584,7 @@ static int config_parse_join_controllers(
> arg_join_controllers = new(char**, 2);
> if (!arg_join_controllers) {
> strv_free(l);
> - return -ENOMEM;
> + return log_oom();
> }
>
> arg_join_controllers[0] = l;
> @@ -598,7 +598,7 @@ static int config_parse_join_controllers(
> t = new0(char**, n+2);
> if (!t) {
> strv_free(l);
> - return -ENOMEM;
> + return log_oom();
> }
>
> n = 0;
> @@ -612,7 +612,7 @@ static int config_parse_join_controllers(
> if (!c) {
> strv_free(l);
> strv_free_free(t);
> - return -ENOMEM;
> + return log_oom();
> }
>
> strv_free(l);
> @@ -624,7 +624,7 @@ static int config_parse_join_controllers(
> if (!c) {
> strv_free(l);
> strv_free_free(t);
> - return -ENOMEM;
> + return log_oom();
> }
>
> t[n++] = c;
> @@ -729,8 +729,10 @@ static int parse_proc_cmdline(void) {
> r = parse_proc_cmdline_word(word);
> free(word);
>
> - if (r < 0)
> + if (r < 0) {
> + log_error("Failed on cmdline argument %s: %s", word, strerror(-r));
> goto finish;
> + }
> }
>
> r = 0;
> @@ -1017,8 +1019,10 @@ static int parse_argv(int argc, char *argv[]) {
> * instead. */
>
> for (a = argv; a < argv + argc; a++)
> - if ((r = parse_proc_cmdline_word(*a)) < 0)
> + if ((r = parse_proc_cmdline_word(*a)) < 0) {
> + log_error("Failed on cmdline argument %s: %s", *a, strerror(-r));
> return r;
> + }
> }
>
> return 0;
> @@ -1293,8 +1297,10 @@ int main(int argc, char *argv[]) {
> }
>
> /* Initialize default unit */
> - if (set_default_unit(SPECIAL_DEFAULT_TARGET) < 0)
> + if (r == set_default_unit(SPECIAL_DEFAULT_TARGET) < 0) {
> + log_error("Failed to set default unit %s: %s", SPECIAL_DEFAULT_TARGET, strerror(-r));
> goto finish;
> + }
>
> /* By default, mount "cpu" and "cpuacct" together */
> arg_join_controllers = new(char**, 2);
> diff --git a/src/core/mount-setup.c b/src/core/mount-setup.c
> index 07794df..c10c6da 100644
> --- a/src/core/mount-setup.c
> +++ b/src/core/mount-setup.c
> @@ -190,8 +190,7 @@ int mount_cgroup_controllers(char ***join_controllers) {
>
> controllers = set_new(string_hash_func, string_compare_func);
> if (!controllers) {
> - r = -ENOMEM;
> - log_error("Failed to allocate controller set.");
> + r = log_oom();
> goto finish;
> }
>
> @@ -262,9 +261,8 @@ int mount_cgroup_controllers(char ***join_controllers) {
>
> options = strv_join(*k, ",");
> if (!options) {
> - log_error("Failed to join options");
> free(controller);
> - r = -ENOMEM;
> + r = log_oom();
> goto finish;
> }
>
> @@ -275,9 +273,8 @@ int mount_cgroup_controllers(char ***join_controllers) {
>
> where = strappend("/sys/fs/cgroup/", options);
> if (!where) {
> - log_error("Failed to build path");
> free(options);
> - r = -ENOMEM;
> + r = log_oom();
> goto finish;
> }
>
> @@ -306,8 +303,7 @@ int mount_cgroup_controllers(char ***join_controllers) {
>
> t = strappend("/sys/fs/cgroup/", *i);
> if (!t) {
> - log_error("Failed to build path");
> - r = -ENOMEM;
> + r = log_oom();
> free(options);
> goto finish;
> }
Lennart
--
Lennart Poettering - Red Hat, Inc.
More information about the systemd-devel
mailing list