[systemd-devel] [PATCH] do not ellipsize cgroup members in full status

[Lukáš Nykrýn]

Lennart Poettering píše v St 16. 01. 2013 v 04:53 +0100:
> On Tue, 15.01.13 10:58, Lukáš Nykrýn (lnykryn at redhat.com) wrote:
> 
> Heya,
> 
> > diff --git a/src/shared/output.h b/src/shared/output.h
> > new file mode 100644
> > index 0000000..0efd430
> > --- /dev/null
> > +++ b/src/shared/output.h
> 
> "output.h" is a bit too generic for my test, maybe a better name would
> be output-mode.h or so?
> 
> >          if (pid == 0)
> > @@ -919,7 +916,7 @@ int get_process_cmdline(pid_t pid, size_t max_length, bool comm_fallback, char *
> >          else {
> >                  char *p;
> >                  if (asprintf(&p, "/proc/%lu/cmdline", (unsigned long) pid) < 0)
> > -                        return -ENOMEM;
> > +                        return log_oom();
> 
> Library calls shouldn't log on their own, except on debug level. Only
> app code should. The line between "library" and "app" in systemd is a
> blurry one as we have everything in the same repo, and link things
> together in varying combinations, but util.c is definitely of the
> "library" category and should hence not log errors directly, not even
> for OOM, and just return ENOMEM;
> 
> >  
> >                  f = fopen(p, "re");
> >                  free(p);
> > @@ -927,47 +924,66 @@ int get_process_cmdline(pid_t pid, size_t max_length, bool comm_fallback, char *
> >  
> >          if (!f)
> >                  return -errno;
> > +        if (max_length == 0) {
> > +                size_t len = 0, alloc = 0;
> > +                while ((c = getc(f)) != EOF) {
> > +                        if(alloc <= len+1) {
> > +                                alloc += 20;
> > +                                k = realloc(r, alloc);
> 
> As I learnt recently realloc() is actually kinda smart internally, and
> will increase the allocated buffer in chunks anyway, so that jumping 20
> bytes at a time here (or even doing exponential increases as a lot of
> code) is actually pretty much unnecessary, and we could just call
> realloc() here always with the byte-exact length we need. Doesn't really
> make anything slower or faster, but certainly makes thing simpler to read...
> 
> > +                                if (k == NULL) {
> > +                                        free(r);
> > +                                        fclose(f);
> > +                                        return log_oom();
> 
> No log_oom() here either, please
> 
> > +                                }
> > +                                r = k;
> > +                        }
> > +                        r[len] = isprint(c) ? c : ' ';
> > +                        r[++len] = 0;
> > +                }
> > +        } else {
> > +                bool space = false;
> > +                size_t left;
> > +                r = new(char, max_length);
> > +                if (!r) {
> > +                        fclose(f);
> > +                        return log_oom();
> > +                }
> 
> Otherwise looks good!
> 
> Lennart
> 

Thanks for your remarks, I hope that I didn't forget about something.

Lukas

-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-systemctl-loginctl-cgls-do-not-ellipsize-cgroup-memb.patch
Type: text/x-patch
Size: 28790 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/systemd-devel/attachments/20130116/c2124665/attachment-0001.bin>