[systemd-devel] [PATCH 2/4] util: add functions getting proc status, maps, limits, cgroup

[Jakub Filak]

On Thu, 2014-11-20 at 14:36 +0100, Lennart Poettering wrote:
> On Wed, 19.11.14 11:01, Jakub Filak (jfilak at redhat.com) wrote:
> 
> > ---
> >  src/shared/util.c    | 13 +++++++++++++
> >  src/shared/util.h    |  4 ++++
> >  src/test/test-util.c | 17 +++++++++++++++++
> >  3 files changed, 34 insertions(+)
> > 
> > diff --git a/src/shared/util.c b/src/shared/util.c
> > index 0166052..d62d90c 100644
> > --- a/src/shared/util.c
> > +++ b/src/shared/util.c
> > @@ -892,6 +892,19 @@ int get_process_root(pid_t pid, char **root) {
> >          return get_process_link_contents(p, root);
> >  }
> >  
> > +#define DEFINE_FN_GET_PROCESS_FULL_FILE(name) \
> > +int get_process_##name(pid_t pid, char **name) { \
> > +        const char *p; \
> > +        assert(pid >= 0); \
> > +        p = procfs_file_alloca(pid, #name); \
> > +        return read_full_file(p, name, /*size*/NULL); \
> > +}
> > +
> > +DEFINE_FN_GET_PROCESS_FULL_FILE(status)
> > +DEFINE_FN_GET_PROCESS_FULL_FILE(maps)
> > +DEFINE_FN_GET_PROCESS_FULL_FILE(limits)
> > +DEFINE_FN_GET_PROCESS_FULL_FILE(cgroup)
> > +
> 
> Please use functions instead of macros wherever that works. 
> 
> Maybe it is sufficient to just provide a single function for all four
> cases that takes an extra argument for the file name to read?
> 
> Maybe:
> 
>         int get_process_proc_file(pid_t pid, const char *filename, char **ret)
> 
> Or so? Given that the files in question are generally just read and
> passed on as is without processing them any further I think it is Ok
> to just provide a single "bulk" call that covers all four cases
> instead of four individual ones.
> 
> Hope that makes sense?
> 

It definitely make sense. I actually wanted to introduce
'get_process_proc_file()' function, but I didn't do that because
'procfs_file_alloca' is a macro and its last argument must be a string
literal, so I couldn't use the following construction:

        p = procfs_file_alloca(pid, filename);

I forgot to mention this fact in the commit message.

New version will arrive soon.

Thanks for the review!


Jakub