Re: [systemd-devel] tmpfiles.d specifier support on argument when operating on files
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
Re: [systemd-devel] tmpfiles.d specifier support on argument when operating on files
On Wed, 18.02.15 18:17, Cristian Rodríguez (crrodrig...@opensuse.org) wrote:
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)) {
I figure it makes sense on more than these, for example the ones that
write strings to files. Also, there's a space missing after the if
and before the first (.
+if(i.argument) {
Similar here.
+_cleanup_free_ char *resolved_iarg = NULL;
+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);
+if(r 0)
+return log_oom();
No need to duplicate the already allocated string again. Just assign
resolved_iarg to i.argument, and free that before.
Lennart
--
Lennart Poettering, Red Hat
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] tmpfiles.d specifier support on argument when operating on files
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
Re: [systemd-devel] tmpfiles.d specifier support on argument when operating on files
On Wed, Feb 18, 2015 at 6:17 PM, Cristian Rodríguez crrodrig...@opensuse.org wrote: Is the attached version cool for you ? Ping ? Any comments about this one ? note that implementing the specifier expansion in all cases for argument does not make sense to me unless I am missing something . ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] tmpfiles.d specifier support on argument when operating on files
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;
+}
A good candidate for log_error_errno().
Lennart
--
Lennart Poettering, Red Hat
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] tmpfiles.d specifier support on argument when operating on files
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. Thanks for taking a look, I will post a complete version later. ;) ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] tmpfiles.d specifier support on argument when operating on files
El 17/02/15 a las 15:38, Cristian Rodríguez escribió:
HI:
In openSUSE, we have per *running kernel* sysctl snippets, currently
implemented as patch on systemd-sysctl.. I 'm currently attempting to
replace it for something sane..like
d /run/sysctl.d
L/run/sysctl.d/00-sysctl.conf-%v ----
/boot/sysctl.conf-%v
BUt argument does not support specifier expasion when dealing with
path names.. would you take patches for this ?
Thanks.
Ok, this patch works for my usecase..
From 10f9c96bb40834191765bd19f313ab17d18e3b31 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Cristian=20Rodr=C3=ADguez?= crrodrig...@opensuse.org
Date: Tue, 17 Feb 2015 17:34:17 -0300
Subject: [PATCH] tmpfiles: add specifier support for the create symlink
argument
---
src/tmpfiles/tmpfiles.c | 6 ++
1 file changed, 6 insertions(+)
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);
+if (r 0) {
+log_error([%s:%u] Failed to replace specifiers: %s, fname, line, path);
+return r;
+}
}
break;
--
2.2.2
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] tmpfiles.d specifier support on argument when operating on files
HI: In openSUSE, we have per *running kernel* sysctl snippets, currently implemented as patch on systemd-sysctl.. I 'm currently attempting to replace it for something sane..like d /run/sysctl.d L/run/sysctl.d/00-sysctl.conf-%v ---- /boot/sysctl.conf-%v BUt argument does not support specifier expasion when dealing with path names.. would you take patches for this ? Thanks. ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
