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.
Zbyszek
_______________________________________________
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
