[systemd-devel] [PATCH] RFC: util: Avoid memory allocations for formatting paths

[Lennart Poettering]

On Wed, 03.04.13 08:27, Holger Freyther (holger at freyther.de) wrote:

> +#define PROCFS_PATH_LEN (sizeof("/proc/")-1 + DECIMAL_STR_MAX(unsigned long))

Even though we use %lu to actually print the PID it's stil in the PID
range, so I'd prefer using DECIMAL_STR_MAX(pid_t) here...

> +#define format_procfs_path(buffer, path, pid) do { \
> +        snprintf(buffer, sizeof(buffer) - 1, "/proc/%lu/%s", (unsigned long) pid, path); \
> +        char_array_0(buffer); } while(0)

Hmm, I'd prefer putting the "do {" and the "} while (0)" on lines of
their own. I mean, macros are unreadable enough as they are, let's make
this a bit nicer...

Also, could you use uppercase chars for the macro name? That indicates a
bit that this is a macro with macro semantics (i.e. people are more
likely to expect the double evaluation of the arguments if it
doesn't look like a function... Also, it wouldn't be as surprising then
that the argument is actually really treated as an array rather than
just a pointer...) [And yes, char_array_0() is badly named, too, it
should really be uppercase...)

Also, given that we rely on the size to be right it might make sense to
add an assert() or so that ensures the value returned by snprintf() is
actually smaller than the size of the array. That assert() should be
good enough to immediately detect at least the case where people pass a
pointer rather than an array as buffer.

Otherwise looks pretty good!

Lennart

-- 
Lennart Poettering - Red Hat, Inc.