[systemd-devel] [PATCH] use "Out of memory." consistantly (or with "\n")
shawn
shawnlandden at gmail.com
Mon Aug 6 11:43:55 PDT 2012
On Mon, 2012-08-06 at 16:39 +0200, Lennart Poettering wrote:
> On Fri, 03.08.12 17:33, shawn (shawnlandden at 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 <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
>
--
-Shawn Landden
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-cgtop-avoid-multiple-error-messages.patch
Type: text/x-patch
Size: 1707 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/systemd-devel/attachments/20120806/ca95f9ae/attachment.bin>
More information about the systemd-devel
mailing list