On Mon, 2012-08-06 at 16:39 +0200, Lennart Poettering wrote:
> On Fri, 03.08.12 17:33, shawn (shawnland...@gmail.com) wrote:
>
> Applied!
>
ha! clang didn't find that careless typo you fixed, while gcc points it
out. I call BS on the "better diagnostics".
> 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
Yeah... I was kinda negligent
I think this is better.
>
> > On Thu, 2012-07-26 at 11:51 +0200, Kay Sievers wrote:
> > > On Thu, Jul 26, 2012 at 1:17 AM, shawn <shawnland...@gmail.com> wrote:
> > > > On Wed, 2012-07-25 at 18:11 +0200, Lennart Poettering wrote:
> > > >> On Wed, 25.07.12 11:24, Kay Sievers (k...@vrfy.org) wrote:
> > > >> > On Wed, Jul 25, 2012 at 6:12 AM, Shawn Landden
> > > >> > <shawnland...@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 <shawnland...@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
>
--
-Shawn Landden
>From e3913d7ef930834cb391e24e57f70a1670436875 Mon Sep 17 00:00:00 2001
From: Shawn Landden <shawnland...@gmail.com>
Date: Mon, 6 Aug 2012 11:17:52 -0700
Subject: [PATCH] cgtop: avoid multiple error messages
---
src/cgtop/cgtop.c | 16 +++++-----------
1 file changed, 5 insertions(+), 11 deletions(-)
diff --git a/src/cgtop/cgtop.c b/src/cgtop/cgtop.c
index 3756328..1fc8808 100644
--- a/src/cgtop/cgtop.c
+++ b/src/cgtop/cgtop.c
@@ -615,11 +615,8 @@ static int parse_argv(int argc, char *argv[]) {
arg_order = ORDER_IO;
break;
- case '?':
- return -EINVAL;
-
default:
- log_error("Unknown option code %c", c);
+ log_error("Unknown option code '%c'", c);
return -EINVAL;
}
}
@@ -664,8 +661,10 @@ int main(int argc, char *argv[]) {
if (t >= last_refresh + arg_delay || immediate_refresh) {
r = refresh(a, b, iteration++);
- if (r < 0)
+ if (r < 0) {
+ log_error("Failed to refresh cgroup data: %s", strerror(-r));
goto finish;
+ }
group_hashmap_clear(b);
@@ -782,10 +781,5 @@ finish:
group_hashmap_free(a);
group_hashmap_free(b);
- if (r < 0) {
- log_error("Exiting with failure: %s", strerror(-r));
- return EXIT_FAILURE;
- }
-
- return EXIT_SUCCESS;
+ return r < 0 ? EXIT_FAILURE : EXIT_SUCCESS;
}
--
1.7.9.5
_______________________________________________
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
