[systemd-devel] journald API backward compatibility
Hello, I have a question regarding journald and backward compatibility. Are there any known issues with accessing journals that were created by different versions of journald? I want to use the latest libsystemd in my project but I might end up reading journals created by an ancient systemd (e.g. systemd-200). Regards, Jakub ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH v2 2/2] coredump: collect all /proc data useful for bug reporting
On Thu, 2014-11-27 at 06:30 +0100, Zbigniew Jędrzejewski-Szmek wrote: > On Tue, Nov 25, 2014 at 07:37:48AM +0100, Jakub Filak wrote: ... > This all looks (and works) great, so I implemented the changes > suggested above and pushed your patch. Please check that I didn't > break anything. > > I also added a follow up commit to use openat to avoid concatenating > paths... I don't think it matters for efficiency, but the code is > imho slightly nicer without the fixed-size buffers. I tested your patches on Fedora Rawhide and it works fantastic. Thank you very much indeed! Regards, Jakub ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH v2 1/2] util: add function getting proc environ
On the contrary of env, the added function returns all characters
cescaped, because it improves reproducibility.
---
src/shared/util.c| 154 ---
src/shared/util.h| 1 +
src/test/test-util.c | 5 ++
3 files changed, 103 insertions(+), 57 deletions(-)
diff --git a/src/shared/util.c b/src/shared/util.c
index 0166052..94e8ed6 100644
--- a/src/shared/util.c
+++ b/src/shared/util.c
@@ -174,6 +174,69 @@ char* first_word(const char *s, const char *word) {
return (char*) p;
}
+static size_t cescape_char(char c, char *buf) {
+char * buf_old = buf;
+
+switch (c) {
+
+case '\a':
+*(buf++) = '\\';
+*(buf++) = 'a';
+break;
+case '\b':
+*(buf++) = '\\';
+*(buf++) = 'b';
+break;
+case '\f':
+*(buf++) = '\\';
+*(buf++) = 'f';
+break;
+case '\n':
+*(buf++) = '\\';
+*(buf++) = 'n';
+break;
+case '\r':
+*(buf++) = '\\';
+*(buf++) = 'r';
+break;
+case '\t':
+*(buf++) = '\\';
+*(buf++) = 't';
+break;
+case '\v':
+*(buf++) = '\\';
+*(buf++) = 'v';
+break;
+case '\\':
+*(buf++) = '\\';
+*(buf++) = '\\';
+break;
+case '"':
+*(buf++) = '\\';
+*(buf++) = '"';
+break;
+case '\'':
+*(buf++) = '\\';
+*(buf++) = '\'';
+break;
+
+default:
+/* For special chars we prefer octal over
+ * hexadecimal encoding, simply because glib's
+ * g_strescape() does the same */
+if ((c < ' ') || (c >= 127)) {
+*(buf++) = '\\';
+*(buf++) = octchar((unsigned char) c >> 6);
+*(buf++) = octchar((unsigned char) c >> 3);
+*(buf++) = octchar((unsigned char) c);
+} else
+*(buf++) = c;
+break;
+}
+
+return buf - buf_old;
+}
+
int close_nointr(int fd) {
assert(fd >= 0);
@@ -892,6 +955,39 @@ int get_process_root(pid_t pid, char **root) {
return get_process_link_contents(p, root);
}
+int get_process_environ(pid_t pid, char **environ) {
+_cleanup_fclose_ FILE *f = NULL;
+_cleanup_free_ char *outcome = NULL;
+int c;
+const char *p;
+size_t allocated = 0, sz = 0;
+
+assert(pid >= 0);
+assert(environ);
+
+p = procfs_file_alloca(pid, "environ");
+
+f = fopen(p, "re");
+if (!f)
+return -errno;
+
+while ((c = fgetc(f)) != EOF) {
+if (!GREEDY_REALLOC(outcome, allocated, sz + 5))
+return -ENOMEM;
+
+if (c == '\0')
+outcome[sz++] = '\n';
+else
+sz += cescape_char(c, outcome + sz);
+}
+
+outcome[sz] = '\0';
+*environ = outcome;
+outcome = NULL;
+
+return 0;
+}
+
char *strnappend(const char *s, const char *suffix, size_t b) {
size_t a;
char *r;
@@ -1271,63 +1367,7 @@ char *cescape(const char *s) {
return NULL;
for (f = s, t = r; *f; f++)
-
-switch (*f) {
-
-case '\a':
-*(t++) = '\\';
-*(t++) = 'a';
-break;
-case '\b':
-*(t++) = '\\';
-*(t++) = 'b';
-break;
-case '\f':
-*(t++) = '\\';
-*(t++) = 'f';
-break;
-case '\n':
-*(t++) = '\\';
-*(t++) = 'n';
-break;
-case '\r':
-*(t++) = '\\';
-*(t++) = 'r';
-break;
-case '\t':
-*(t++) = '\\';
-*(t++) = 't';
-break;
-case '\v':
-*(t++) = '\\';
-
[systemd-devel] [PATCH v2 0/2] coredump: add journal fields useful for bug reporting
'util: add functions getting proc status, maps, limits, cgroup' I dropped this patch, because we just need to read full proc file and 'get_process_proc_file()' function is not implementable without adding a wrapper function to procfs_file_alloca() macro. 'util: add function getting proc environ' get_process_environ() function does not use unnecessary buffer, allocates 5 more bytes in each loop cycle and writes escaped bytes directly to the result memory. 'coredump: collect all /proc data useful for bug reporting' compose_open_fds() function uses open_memstream() and tries to open only existing files in /proc/[pid]/fd/ limits, status, maps and cgroups files are obtained by calling read_full_file with path returned by procfs_file_alloca(). I tested these patches on Fedora Rawhide. Jakub Filak (2): util: add function getting proc environ coredump: collect all /proc data useful for bug reporting src/journal/coredump.c | 156 - src/shared/util.c | 154 ++-- src/shared/util.h | 1 + src/test/test-util.c | 5 ++ 4 files changed, 257 insertions(+), 59 deletions(-) -- 1.8.3.1 ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH v2 2/2] coredump: collect all /proc data useful for bug reporting
/proc/[pid]:
- status
- maps
- limits
- cgroup
- cwd
- root
- environ
- fd/ & fdinfo/ joined in open_fds
---
src/journal/coredump.c | 156 -
1 file changed, 154 insertions(+), 2 deletions(-)
diff --git a/src/journal/coredump.c b/src/journal/coredump.c
index 26a2010..58012d6 100644
--- a/src/journal/coredump.c
+++ b/src/journal/coredump.c
@@ -36,6 +36,7 @@
#include "log.h"
#include "util.h"
+#include "fileio.h"
#include "strv.h"
#include "macro.h"
#include "mkdir.h"
@@ -461,24 +462,107 @@ static int allocate_journal_field(int fd, size_t size,
char **ret, size_t *ret_s
return 0;
}
+/* Joins /proc/[pid]/fd/ and /proc/[pid]/fdinfo/ into the following lines:
+ * 0:/dev/pts/23
+ * pos:0
+ * flags: 012
+ *
+ * 1:/dev/pts/23
+ * pos:0
+ * flags: 012
+ *
+ * 2:/dev/pts/23
+ * pos:0
+ * flags: 012
+ * EOF
+ */
+static int compose_open_fds(pid_t pid, char **open_fds) {
+_cleanup_fclose_ FILE *stream = NULL;
+_cleanup_free_ char *outcome = NULL;
+char path[PATH_MAX], line[LINE_MAX];
+size_t ignored_size;
+const char *fddelim = "";
+struct dirent *dent = NULL;
+_cleanup_closedir_ DIR *proc_fd_dir = NULL;
+int r = 0;
+
+assert(pid >= 0);
+assert(open_fds != NULL);
+
+sprintf(path, "/proc/"PID_FMT"/fd", pid);
+proc_fd_dir = opendir(path);
+
+if (proc_fd_dir == NULL)
+return -ENOENT;
+
+stream = open_memstream(&outcome, &ignored_size);
+
+for (dent = readdir(proc_fd_dir); dent != NULL; dent =
readdir(proc_fd_dir)) {
+_cleanup_free_ char *fdname = NULL;
+_cleanup_fclose_ FILE *fdinfo = NULL;
+
+if (dent->d_name[0] == '.' || strcmp(dent->d_name, "..") == 0)
+continue;
+
+/* Too long path is unlikely a path to valid file descriptor
in /proc/[pid]/fd */
+/* Skip it. */
+r = snprintf(path, sizeof(path), "/proc/"PID_FMT"/fd/%s", pid,
dent->d_name);
+if (r >= (int)sizeof(path))
+continue;
+
+r = readlink_malloc(path, &fdname);
+if (r < 0)
+return r;
+
+fprintf(stream, "%s%s:%s\n", fddelim, dent->d_name, fdname);
+fddelim = "\n";
+
+/* Use the directory entry from /proc/[pid]/fd with
/proc/[pid]/fdinfo */
+
+/* Too long path is unlikely a path to valid file descriptor
info in /proc/[pid]/fdinfo */
+/* Skip it. */
+r = snprintf(path, sizeof(path), "/proc/"PID_FMT"/fdinfo/%s",
pid, dent->d_name);
+if (r >= (int)sizeof(path))
+continue;
+
+fdinfo = fopen(path, "re");
+if (fdinfo == NULL)
+continue;
+
+while(fgets(line, sizeof(line), fdinfo) != NULL)
+fprintf(stream, "%s%s", strchr(line, '\n') == NULL ?
"\n" : "", line);
+}
+
+fclose(stream);
+stream = NULL;
+
+*open_fds = outcome;
+outcome = NULL;
+
+return 0;
+}
+
int main(int argc, char* argv[]) {
_cleanup_free_ char *core_pid = NULL, *core_uid = NULL, *core_gid =
NULL, *core_signal = NULL,
*core_timestamp = NULL, *core_comm = NULL, *core_exe = NULL,
*core_unit = NULL,
*core_session = NULL, *core_message = NULL, *core_cmdline =
NULL, *coredump_data = NULL,
-*core_slice = NULL, *core_cgroup = NULL, *core_owner_uid =
NULL,
+*core_slice = NULL, *core_cgroup = NULL, *core_owner_uid =
NULL, *core_open_fds = NULL,
+*core_proc_status = NULL, *core_proc_maps = NULL,
*core_proc_limits = NULL, *core_proc_cgroup = NULL,
+*core_cwd = NULL, *core_root = NULL, *core_environ = NULL,
*exe = NULL, *comm = NULL, *filename = NULL;
const char *info[_INFO_LEN];
_cleanup_close_ int coredump_fd = -1;
-struct iovec iovec[18];
+struct iovec iovec[26];
off_t coredump_size;
int r, j = 0;
uid_t uid, owner_uid;
gid_t gid;
pid_t pid;
char *t;
+const char *p;
/* Make sure we never enter a loop */
prctl(PR_SET_DUMPABLE, 0);
@@ -638,6 +722,74 @@ int main(int argc, char* argv[]) {
IOVEC_SET_STRING(iovec[j++], core_cgroup);
}
+if (compose_open_fds(pid, &t) >= 0) {
+core_open_fds = strappend("COREDUMP_OPEN_FDS=", t);
+free(t);
+
+if (core_open_fds)
+IOVEC_SET_STRING(iovec[j++], core_open_fds);
+}
+
+p = procfs_file_alloca(pid, "status");
+if (read_full_file(p, &t, NULL) >= 0) {
+core_proc_stat
Re: [systemd-devel] [PATCH 4/4] coredump: collect all /proc data useful for bug reporting
On Fri, 2014-11-21 at 21:14 +0100, Lennart Poettering wrote:
> On Wed, 19.11.14 11:01, Jakub Filak (jfi...@redhat.com) wrote:
>
> > +/* Joins /proc/[pid]/fd/ and /proc/[pid]/fdinfo/ into the following lines:
> > + *
> > + * 0:/dev/pts/23
> > + * pos:0
> > + * flags: 012
> > + * 1:/dev/pts/23
> > + * pos:0
> > + * flags: 012
> > + * 2:/dev/pts/23
>
> Hmm, I'd prefer a format here that is more easily reversible. For
> example, adding an extra newline between the fdinfo items would be a
> good start.
>
I took this format from ABRT. I will add the extra blank line.
> > + *
> > + */
> > +static int compose_open_fds(pid_t pid, char **open_fds) {
> > +const char *fd_name = NULL, *fdinfo_name = NULL;
>
> const? why?
Not sure, typo? Lack of caffeine?
>
> > +char *outcome = NULL;
> > +size_t len = 0, allocated = 0;
> > +char line[LINE_MAX];
> > +unsigned fd = 0;
> > +int r = 0;
> > +
> > +assert(pid >= 0);
> > +
> > +fd_name = alloca(strlen("/proc//fd/") + DECIMAL_STR_MAX(pid_t) +
> > DECIMAL_STR_MAX(unsigned) + 1);
>
> ^^^
>
> unsigned? an fd is an int!
Thanks, I overlooked it.
> > +fdinfo_name = alloca(strlen("/proc//fdinfo/") +
> > DECIMAL_STR_MAX(pid_t) + DECIMAL_STR_MAX(unsigned) + 1);
>
> same here.
>
> The sizes you allocate here are fixed. I'd really prefer if you'd
> allocate these as normal arrays instead of alloca(). alloca() is a
> useful tool, but we should use it only when normal arrays aren't good
> denough, but not otherwise.
>
> Also note that alloca() cannot be mixed with function calls in the
> same line. strlen() is a function call (though one that today's gcc
> actually is smart enough to optimize away at compile time if you
> invoke it on a literal string).
>
> Hence, please use this instead:
>
> char fd_name[sizeof("/proc/")-1 + DECIMAL_STR_MAX(pid_t) + sizeof("/fd/")-1 +
> DECIMAL_STR_MAX(int) + 1];
OK, I thought systemd prefers alloca(). I was inspired by
procfs_file_alloca().
> > +
> > +while (fd <= 9) {
>
> Oh no, this is not OK!
>
> We shouldn't iterate though all thinkable fds, that's bad code. Please
> iterate through /proc/$PID/fd/ and just operate on fds that are
> actually there.
>
OK.
Just a note, it iterates until it finds the first non-existing fd.
> > +_cleanup_free_ char *name = NULL;
> > +_cleanup_fclose_ FILE *fdinfo = NULL;
> > +
> > +sprintf((char *)fd_name, "/proc/"PID_FMT"/fd/%u", pid, fd);
>
> Hmm, first you declare the string as "const", then you cast this away?
> This is usually a good indication that something is really wrong...
Very bad! I hate code where people cast from const to non-const. What I
was thinking about while writing this patch?
>
> > +r = readlink_malloc(fd_name, &name);
> > +if (r < 0) {
> > +if (r == -ENOENT) {
> > +*open_fds = outcome;
> > +r = 0;
> > +}
> > +else
> > +free(outcome);
> > +
> > +break;
> > +}
> > +
> > +if (!GREEDY_REALLOC(outcome, allocated, len + strlen(name)
> > + DECIMAL_STR_MAX(unsigned) + 3))
> > +return -ENOMEM;
> > +
> > +len += sprintf(outcome + len, "%u:%s\n", fd, name);
> > +++fd;
> > +
> > +sprintf((char *)fdinfo_name, "/proc/"PID_FMT"/fdinfo/%u",
> > pid, fd);
> > +fdinfo = fopen(fdinfo_name, "r");
> > +if (fdinfo == NULL)
> > +continue;
> > +
> > +while(fgets(line, sizeof(line), fdinfo) != NULL) {
> > +if (!GREEDY_REALLOC(outcome, allocated, len +
> > strlen(line) + 2))
> > +return -ENOMEM;
> > +
> > +len += sprintf(outcome + len, "%s", line);
> > +if (strchr(line, '\n') == NULL) {
> > +outcome[len++] = '\n';
> > +outcome[len] = '\0';
> > +}
>
> > +}
>
> I think using libc's open_memstream() and then simply writing to it
> would be a *ton* prettier than this.
>
Fabulous! I think so too. I wasn't allowed to use such a construction in
other projects.
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
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
[systemd-devel] [PATCH 2/4] util: add functions getting proc status, maps, limits, cgroup
---
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
[systemd-devel] [PATCH 1/4] util: add functions getting proc cwd and root
/proc/[pid]/cwd and /proc/[pid]/root are symliks to corresponding
directories
The added functions returns values of that symlinks.
---
src/shared/util.c| 39 +++
src/shared/util.h| 2 ++
src/test/test-util.c | 13 -
3 files changed, 49 insertions(+), 5 deletions(-)
diff --git a/src/shared/util.c b/src/shared/util.c
index eeced47..0166052 100644
--- a/src/shared/util.c
+++ b/src/shared/util.c
@@ -797,19 +797,30 @@ int get_process_capeff(pid_t pid, char **capeff) {
return get_status_field(p, "\nCapEff:", capeff);
}
+static int get_process_link_contents(const char *proc_file, char **name) {
+int r;
+
+assert(proc_file);
+assert(name);
+
+r = readlink_malloc(proc_file, name);
+if (r < 0)
+return r == -ENOENT ? -ESRCH : r;
+
+return 0;
+}
+
int get_process_exe(pid_t pid, char **name) {
const char *p;
char *d;
int r;
assert(pid >= 0);
-assert(name);
p = procfs_file_alloca(pid, "exe");
-
-r = readlink_malloc(p, name);
+r = get_process_link_contents(p, name);
if (r < 0)
-return r == -ENOENT ? -ESRCH : r;
+return r;
d = endswith(*name, " (deleted)");
if (d)
@@ -861,6 +872,26 @@ int get_process_gid(pid_t pid, gid_t *gid) {
return get_process_id(pid, "Gid:", gid);
}
+int get_process_cwd(pid_t pid, char **cwd) {
+const char *p;
+
+assert(pid >= 0);
+
+p = procfs_file_alloca(pid, "cwd");
+
+return get_process_link_contents(p, cwd);
+}
+
+int get_process_root(pid_t pid, char **root) {
+const char *p;
+
+assert(pid >= 0);
+
+p = procfs_file_alloca(pid, "root");
+
+return get_process_link_contents(p, root);
+}
+
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 835fee4..fc59481 100644
--- a/src/shared/util.h
+++ b/src/shared/util.h
@@ -295,6 +295,8 @@ int get_process_exe(pid_t pid, char **name);
int get_process_uid(pid_t pid, uid_t *uid);
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);
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 01b0192..7bf8ff6 100644
--- a/src/test/test-util.c
+++ b/src/test/test-util.c
@@ -490,13 +490,14 @@ 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;
+_cleanup_free_ char *a = NULL, *c = NULL, *d = NULL, *f = NULL, *i =
NULL, *cwd = NULL, *root = NULL;
unsigned long long b;
pid_t e;
uid_t u;
gid_t g;
dev_t h;
int r;
+pid_t me;
if (stat("/proc/1/comm", &st) == 0) {
assert_se(get_process_comm(1, &a) >= 0);
@@ -532,6 +533,16 @@ static void test_get_process_comm(void) {
log_info("pid1 gid: "GID_FMT, g);
assert_se(g == 0);
+me = getpid();
+
+r = get_process_cwd(me, &cwd);
+assert_se(r >= 0 || r == -EACCES);
+log_info("pid1 cwd: '%s'", cwd);
+
+r = get_process_root(me, &root);
+assert_se(r >= 0 || r == -EACCES);
+log_info("pid1 root: '%s'", root);
+
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
[systemd-devel] [PATCH 4/4] coredump: collect all /proc data useful for bug reporting
/proc/[pid]:
- status
- maps
- limits
- cgroup
- cwd
- root
- environ
- fd/ & fdinfo/ joined in open_fds
---
src/journal/coredump.c | 137 -
1 file changed, 135 insertions(+), 2 deletions(-)
diff --git a/src/journal/coredump.c b/src/journal/coredump.c
index 26a2010..1b04105 100644
--- a/src/journal/coredump.c
+++ b/src/journal/coredump.c
@@ -461,18 +461,87 @@ static int allocate_journal_field(int fd, size_t size,
char **ret, size_t *ret_s
return 0;
}
+/* Joins /proc/[pid]/fd/ and /proc/[pid]/fdinfo/ into the following lines:
+ *
+ * 0:/dev/pts/23
+ * pos:0
+ * flags: 012
+ * 1:/dev/pts/23
+ * pos:0
+ * flags: 012
+ * 2:/dev/pts/23
+ *
+ */
+static int compose_open_fds(pid_t pid, char **open_fds) {
+const char *fd_name = NULL, *fdinfo_name = NULL;
+char *outcome = NULL;
+size_t len = 0, allocated = 0;
+char line[LINE_MAX];
+unsigned fd = 0;
+int r = 0;
+
+assert(pid >= 0);
+
+fd_name = alloca(strlen("/proc//fd/") + DECIMAL_STR_MAX(pid_t) +
DECIMAL_STR_MAX(unsigned) + 1);
+fdinfo_name = alloca(strlen("/proc//fdinfo/") + DECIMAL_STR_MAX(pid_t)
+ DECIMAL_STR_MAX(unsigned) + 1);
+
+while (fd <= 9) {
+_cleanup_free_ char *name = NULL;
+_cleanup_fclose_ FILE *fdinfo = NULL;
+
+sprintf((char *)fd_name, "/proc/"PID_FMT"/fd/%u", pid, fd);
+r = readlink_malloc(fd_name, &name);
+if (r < 0) {
+if (r == -ENOENT) {
+*open_fds = outcome;
+r = 0;
+}
+else
+free(outcome);
+
+break;
+}
+
+if (!GREEDY_REALLOC(outcome, allocated, len + strlen(name) +
DECIMAL_STR_MAX(unsigned) + 3))
+return -ENOMEM;
+
+len += sprintf(outcome + len, "%u:%s\n", fd, name);
+++fd;
+
+sprintf((char *)fdinfo_name, "/proc/"PID_FMT"/fdinfo/%u", pid,
fd);
+fdinfo = fopen(fdinfo_name, "r");
+if (fdinfo == NULL)
+continue;
+
+while(fgets(line, sizeof(line), fdinfo) != NULL) {
+if (!GREEDY_REALLOC(outcome, allocated, len +
strlen(line) + 2))
+return -ENOMEM;
+
+len += sprintf(outcome + len, "%s", line);
+if (strchr(line, '\n') == NULL) {
+outcome[len++] = '\n';
+outcome[len] = '\0';
+}
+}
+}
+
+return r;
+}
+
int main(int argc, char* argv[]) {
_cleanup_free_ char *core_pid = NULL, *core_uid = NULL, *core_gid =
NULL, *core_signal = NULL,
*core_timestamp = NULL, *core_comm = NULL, *core_exe = NULL,
*core_unit = NULL,
*core_session = NULL, *core_message = NULL, *core_cmdline =
NULL, *coredump_data = NULL,
-*core_slice = NULL, *core_cgroup = NULL, *core_owner_uid =
NULL,
+*core_slice = NULL, *core_cgroup = NULL, *core_owner_uid =
NULL, *core_open_fds = NULL,
+*core_proc_status = NULL, *core_proc_maps = NULL,
*core_proc_limits = NULL, *core_proc_cgroup = NULL,
+*core_cwd = NULL, *core_root = NULL, *core_environ = NULL,
*exe = NULL, *comm = NULL, *filename = NULL;
const char *info[_INFO_LEN];
_cleanup_close_ int coredump_fd = -1;
-struct iovec iovec[18];
+struct iovec iovec[26];
off_t coredump_size;
int r, j = 0;
uid_t uid, owner_uid;
@@ -638,6 +707,70 @@ int main(int argc, char* argv[]) {
IOVEC_SET_STRING(iovec[j++], core_cgroup);
}
+if (compose_open_fds(pid, &t) >= 0) {
+core_open_fds = strappend("COREDUMP_OPEN_FDS=", t);
+free(t);
+
+if (core_open_fds)
+IOVEC_SET_STRING(iovec[j++], core_open_fds);
+}
+
+if (get_process_status(pid, &t) >= 0) {
+core_proc_status = strappend("COREDUMP_PROC_STATUS=", t);
+free(t);
+
+if (core_proc_status)
+IOVEC_SET_STRING(iovec[j++], core_proc_status);
+}
+
+if (get_process_maps(pid, &t) >= 0) {
+core_proc_maps = strappend("COREDUMP_PROC_MAPS=", t);
+free(t);
+
+if (core_proc_maps)
+IOVEC_SET_STRING(iovec[j++], core_proc_maps);
+}
+
+if (get_process_limits(pid, &t) >= 0) {
+core_proc_limits = strappend("COREDUMP_PROC_LIMITS=", t);
+free(t);
+
+if (core_proc_limits)
+
[systemd-devel] [PATCH 0/4] coredump: add journal fields useful for bug reporting
Hello, these patches add several new fields to systemd-coredump journal messages. The added fields should enable ABRT to report core files from systemd-journald to various bug tracking systems. The added fields: - COREDUMP_PROC_STATUS : a copy of /proc/[pid]/status - COREDUMP_PROC_MAPS : a copy of /proc/[pid]/maps - COREDUMP_PROC_LIMITS : a copy of /proc/[pid]/limits - COREDUMP_PROC_CGROUP : a copy of /proc/[pid]/cgroup - COREDUMP_ENVIRON : an escaped copy of /proc/[pid]/environ - COREDUMP_CWD : readlink /proc/[pid]/cwd - COREDUMP_ROOT: readlink /proc/[pid]/root - COREDUMP_OPEN_FDS: readlink /proc/[pid]/fd/* + /proc/[pid]/fdinfo/* Except COREDUMP_OPEN_FDS, the functions getting contents of the fields are added to util.c, because I think these functions can be reused. COREDUMP_OPEN_FDS filed is something special, hence I added it to journal/coredump.c I split the patch set into four patches to better comprehend my intentions. There was a short discussion about this topic in September: http://lists.freedesktop.org/archives/systemd-devel/2014-September/022864.html Jakub Filak (4): util: add functions getting proc cwd and root util: add functions getting proc status, maps, limits, cgroup util: add function getting proc environ coredump: collect all /proc data useful for bug reporting src/journal/coredump.c | 137 +++- src/shared/util.c | 212 +++-- src/shared/util.h | 7 ++ src/test/test-util.c | 34 +++- 4 files changed, 326 insertions(+), 64 deletions(-) -- 1.8.3.1 ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH 3/4] util: add function getting proc environ
On the contrary of env, the added function returns all characters
cescaped, because it improves reproducibility.
---
src/shared/util.c| 160 +--
src/shared/util.h| 1 +
src/test/test-util.c | 6 +-
3 files changed, 109 insertions(+), 58 deletions(-)
diff --git a/src/shared/util.c b/src/shared/util.c
index d62d90c..448efa5 100644
--- a/src/shared/util.c
+++ b/src/shared/util.c
@@ -174,6 +174,69 @@ char* first_word(const char *s, const char *word) {
return (char*) p;
}
+static size_t cescape_char(char c, char *buf) {
+char * buf_old = buf;
+
+switch (c) {
+
+case '\a':
+*(buf++) = '\\';
+*(buf++) = 'a';
+break;
+case '\b':
+*(buf++) = '\\';
+*(buf++) = 'b';
+break;
+case '\f':
+*(buf++) = '\\';
+*(buf++) = 'f';
+break;
+case '\n':
+*(buf++) = '\\';
+*(buf++) = 'n';
+break;
+case '\r':
+*(buf++) = '\\';
+*(buf++) = 'r';
+break;
+case '\t':
+*(buf++) = '\\';
+*(buf++) = 't';
+break;
+case '\v':
+*(buf++) = '\\';
+*(buf++) = 'v';
+break;
+case '\\':
+*(buf++) = '\\';
+*(buf++) = '\\';
+break;
+case '"':
+*(buf++) = '\\';
+*(buf++) = '"';
+break;
+case '\'':
+*(buf++) = '\\';
+*(buf++) = '\'';
+break;
+
+default:
+/* For special chars we prefer octal over
+ * hexadecimal encoding, simply because glib's
+ * g_strescape() does the same */
+if ((c < ' ') || (c >= 127)) {
+*(buf++) = '\\';
+*(buf++) = octchar((unsigned char) c >> 6);
+*(buf++) = octchar((unsigned char) c >> 3);
+*(buf++) = octchar((unsigned char) c);
+} else
+*(buf++) = c;
+break;
+}
+
+return buf - buf_old;
+}
+
int close_nointr(int fd) {
assert(fd >= 0);
@@ -905,6 +968,45 @@ DEFINE_FN_GET_PROCESS_FULL_FILE(maps)
DEFINE_FN_GET_PROCESS_FULL_FILE(limits)
DEFINE_FN_GET_PROCESS_FULL_FILE(cgroup)
+int get_process_environ(pid_t pid, char **environ) {
+_cleanup_fclose_ FILE *f = NULL;
+_cleanup_free_ char *outcome = NULL;
+int c;
+const char *p;
+char escaped[4];
+size_t allocated = 0, sz = 0, escaped_len = 0;
+
+assert(pid >= 0);
+assert(environ);
+
+p = procfs_file_alloca(pid, "environ");
+
+f = fopen(p, "re");
+if (!f)
+return -errno;
+
+while ((c = fgetc(f)) != EOF) {
+if (c == '\0') {
+escaped[0] = '\n';
+escaped_len = 1;
+}
+else
+escaped_len = cescape_char(c, escaped);
+
+if (!GREEDY_REALLOC(outcome, allocated, sz + escaped_len + 1))
+return -ENOMEM;
+
+memcpy(outcome + sz, escaped, escaped_len);
+sz += escaped_len;
+}
+
+outcome[sz] = '\0';
+*environ = outcome;
+outcome = NULL;
+
+return 0;
+}
+
char *strnappend(const char *s, const char *suffix, size_t b) {
size_t a;
char *r;
@@ -1284,63 +1386,7 @@ char *cescape(const char *s) {
return NULL;
for (f = s, t = r; *f; f++)
-
-switch (*f) {
-
-case '\a':
-*(t++) = '\\';
-*(t++) = 'a';
-break;
-case '\b':
-*(t++) = '\\';
-*(t++) = 'b';
-break;
-case '\f':
-*(t++) = '\\';
-*(t++) = 'f';
-break;
-case '\n':
-*(t++) = '\\';
-*(t++) = 'n';
-break;
-case '\r':
-*(t++) = '\\';
-*(t++) = 'r';
-
Re: [systemd-devel] Add more information to coredump related journald etnries
On Fri, 2014-09-05 at 19:06 +0200, Zbigniew Jędrzejewski-Szmek wrote: > On Fri, Sep 05, 2014 at 06:35:19PM +0200, Lennart Poettering wrote: > > On Fri, 05.09.14 01:40, Jakub Filak (jfi...@redhat.com) wrote: > > > > > Hello systemd, > > > > > > I’m a ABRT developer and I’d like to replace ABRT’s coredumper with > > > systemd-coredump and read coredumps from systemd-journal. In order > > > to achieve my goal, I need to extend the set of captured > > > /proc/$PID/[files] > > > by systemd-coredump, because we attach those files in Bugzilla bugs. > > > > > > Files from /proc/PID needed by ABRT: > > > status > > > maps > > > limits > > > cgroup > > > open_fds > > > environ > > > cmdline > > > cwd > > > rootdir > > > > > > > > > Do you have any objections to my plan or hints to achieve my goal? > > > I’d be grateful for any feedback you may have. > > > > Sounds like good things to add, happy to take a patch. the cmdline and > > cgroup we already have (at least in the journal, not on the coredump > > files though -- where we only attach the stuff we get racefreely > > passed in from the kernel, not the stuff we have to read from /proc, > > potentially racefully, at least currently). Attaching all data on the > > coredumps as xattrs should be conceptually OK, but this might not end > > up working due to size limits on xattr imposed by ext4? if the "maps" > > for example might grow large we might not be able to use xattrs to > > store this in. If we cannot store this in xattrs I think we should > > prefer to not invent another storage but simply query the journal for > > the additional metdata. > > I think it is preferable to query the journal always and use is the > primary source of information, rather than enumerating files in > /var/lib/systemd/coredump/. The journal might contain information > about more dumped cores, since a) the user might disable saving cores > to /var/lib/systemd/coredump/, b) some core might be too large to > save, c) there might not be enough disk space. In any of those cases, > the journal might contain a useful traceback that we generated > on-the-fly. I'd prefer to work with abrtd developers to make this > information contain everything they need, rather than doing the > reverse and starting with the core and going back to the journal to > find missing information. > > Zbyszek Lennart, Zbyszek, Thank you for your quick replies! My original intention was much less ambitious. I wanted to save the files in journald fields (_PROC_PID_STATUS, ...) and implement a tool based on Examples at: http://www.freedesktop.org/software/systemd/man/sd_journal_get_fd.html In the first version, the tool would wait for a new coredump and create same data directory as ABRT's coredumper (copy the journald message to disk). Jakub ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] Add more information to coredump related journald etnries
Hello systemd, I’m a ABRT developer and I’d like to replace ABRT’s coredumper with systemd-coredump and read coredumps from systemd-journal. In order to achieve my goal, I need to extend the set of captured /proc/$PID/[files] by systemd-coredump, because we attach those files in Bugzilla bugs. Files from /proc/PID needed by ABRT: status maps limits cgroup open_fds environ cmdline cwd rootdir Do you have any objections to my plan or hints to achieve my goal? I’d be grateful for any feedback you may have. Yours sincerely, Jakub Filak Software Engineer at Red Hat in Brno ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
