[systemd-devel] [PATCH 4/4] coredump: collect all /proc data useful for bug reporting

[Lennart Poettering]

On Mon, 24.11.14 11:01, Jakub Filak (jfilak at redhat.com) wrote:

> > 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().

Well, this is a pseudo-function implemented as macro. In a pseudo-function we can easily
return alloca() allocated buffers, but of course no normal
stack-allocated arrays.

> 
> > > +
> > > +        while (fd <= 99999) {
> > 
> > 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.

That end condition is wrong too, as commonly there are "holes" in the
allocation because people may close fds at any times, and keep fds
with later numbers around.

> > 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.

open_memstream() is even POSIX! For us, GNU and Linux-specific APIs
are fine too, and POSIX is absolutely baseline...

Lennart

-- 
Lennart Poettering, Red Hat