On Mon, Mar 02, 2015 at 08:25:47PM +0100, Zbigniew Jędrzejewski-Szmek wrote: > On Wed, Feb 18, 2015 at 06:17:17PM -0300, Cristian Rodríguez wrote: > > El 18/02/15 a las 07:10, Lennart Poettering escribió: > > >On Tue, 17.02.15 17:35, Cristian Rodríguez (crrodrig...@opensuse.org) > > >wrote: > > > > > >Please fix this for all arguments, not just symlinks. > > > > > >>diff --git a/src/tmpfiles/tmpfiles.c b/src/tmpfiles/tmpfiles.c > > >>index c948d4d..1b35b8e 100644 > > >>--- a/src/tmpfiles/tmpfiles.c > > >>+++ b/src/tmpfiles/tmpfiles.c > > >>@@ -1590,6 +1590,12 @@ static int parse_line(const char *fname, unsigned > > >>line, const char *buffer) { > > >> i.argument = strappend("/usr/share/factory/", > > >> i.path); > > >> if (!i.argument) > > >> return log_oom(); > > >>+ } else { > > >>+ r = specifier_printf(i.argument, > > >>specifier_table, NULL, &i.argument); > > > > > >Here's a memory leak, you need to free the old i.argument. > > > > > >Indentation! Please have a look at CODING_STYLE. You need to indent by > > >8ch. 4ch indenting is not acceptable. > > > > > >>+ if (r < 0) { > > >>+ log_error("[%s:%u] Failed to replace specifiers: > > >>%s", fname, line, path); > > >>+ return r; > > >>+ } > > > > > > > HI again: > > > > Is the attached version cool for you ? > Hi, > > sorry for the delay. We seem to have trouble getting all patches > reviewed recently. Hopefully this is just > conferences/vacations/fedora-alpha-releases, and things will > return to normal. > > > From ee8e4f440def745b6f0655b897e65d48321e46c5 Mon Sep 17 00:00:00 2001 > > From: =?UTF-8?q?Cristian=20Rodr=C3=ADguez?= <crrodrig...@opensuse.org> > > Date: Wed, 18 Feb 2015 18:09:16 -0300 > > Subject: [PATCH] tmpfiles: implement specifier substitution for file > > "argument" > > > > Only for L and C types where it makes sense. > > --- > > src/tmpfiles/tmpfiles.c | 12 ++++++++++++ > > 1 file changed, 12 insertions(+) > > > > diff --git a/src/tmpfiles/tmpfiles.c b/src/tmpfiles/tmpfiles.c > > index 88ba7e4..6de477b 100644 > > --- a/src/tmpfiles/tmpfiles.c > > +++ b/src/tmpfiles/tmpfiles.c > > @@ -1568,6 +1568,18 @@ static int parse_line(const char *fname, unsigned > > line, const char *buffer) { > > } > > } > > > > + if(IN_SET(i.type, CREATE_SYMLINK, COPY_FILES)) { > Please add a space after if and for... > > > + if(i.argument) { > > + _cleanup_free_ char *resolved_iarg = NULL; > ...and empty line after variable declarations. > > > + r = specifier_printf(i.argument, specifier_table, > > NULL, &resolved_iarg); > > + if(r < 0) > > + return log_error_errno(r, "[%s:%u] Failed > > to replace specifiers: %s", fname, line, path); > > + r = free_and_strdup(&i.argument, resolved_iarg); > There's no need to to use free_and_strdup here. resolved_arg was just > newly allocated, so it can be used without any strdup. > > > + if(r < 0) > > + return log_oom(); > > + } > > + } > > + > > switch (i.type) { > > Otherwise looks OK.
Ping? Zbyszek _______________________________________________ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel