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

2014-11-21 Thread Jakub Filak 
On Thu, 2014-11-20 at 14:36 +0100, Lennart Poettering wrote:
 On Wed, 19.11.14 11:01, Jakub Filak (jfi...@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

___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

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

2014-11-20 Thread Lennart Poettering 
On Wed, 19.11.14 11:01, Jakub Filak (jfi...@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?

Lennart

-- 
Lennart Poettering, Red Hat
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

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

2014-11-19 Thread Jakub Filak 
---
 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)
+
 char *strnappend(const char *s, const char *suffix, size_t b) {
 size_t a;
 char *r;
diff --git a/src/shared/util.h b/src/shared/util.h
index fc59481..2c9e4fe 100644
--- a/src/shared/util.h
+++ b/src/shared/util.h
@@ -297,6 +297,10 @@ int get_process_gid(pid_t pid, gid_t *gid);
 int get_process_capeff(pid_t pid, char **capeff);
 int get_process_cwd(pid_t pid, char **cwd);
 int get_process_root(pid_t pid, char **root);
+int get_process_status(pid_t pid, char **status);
+int get_process_maps(pid_t pid, char **maps);
+int get_process_limits(pid_t pid, char **limits);
+int get_process_cgroup(pid_t pid, char **cgroup);
 
 char hexchar(int x) _const_;
 int unhexchar(char c) _const_;
diff --git a/src/test/test-util.c b/src/test/test-util.c
index 7bf8ff6..f7c0210 100644
--- a/src/test/test-util.c
+++ b/src/test/test-util.c
@@ -491,6 +491,7 @@ static void test_u64log2(void) {
 static void test_get_process_comm(void) {
 struct stat st;
 _cleanup_free_ char *a = NULL, *c = NULL, *d = NULL, *f = NULL, *i = 
NULL, *cwd = NULL, *root = NULL;
+_cleanup_free_ char *s = NULL, *l = NULL, *m = NULL, *cg = NULL;
 unsigned long long b;
 pid_t e;
 uid_t u;
@@ -543,6 +544,22 @@ static void test_get_process_comm(void) {
 assert_se(r = 0 || r == -EACCES);
 log_info(pid1 root: '%s', root);
 
+r = get_process_status(me, s);
+assert_se(r = 0 || r == -EACCES);
+log_info(pid1 strlen(status): '%zd', strlen(s));
+
+r = get_process_maps(me, m);
+assert_se(r = 0 || r == -EACCES);
+log_info(pid1 strlen(maps): '%zd', strlen(m));
+
+r = get_process_limits(me, l);
+assert_se(r = 0 || r == -EACCES);
+log_info(pid1 strlen(limits): '%zd', strlen(l));
+
+r = get_process_cgroup(me, cg);
+assert_se(r = 0 || r == -EACCES);
+log_info(pid1 strlen(cgroup): '%zd', strlen(cg));
+
 assert_se(get_ctty_devnr(1, h) == -ENOENT);
 
 getenv_for_pid(1, PATH, i);
-- 
1.8.3.1

___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel