[systemd-devel] [PATCH] timedated: support split usr v3

[Shawn Landden]

From: Shawn Paul Landden 

The current Debian solution to this is really ugly, and I would rather
have them use the correct patch even if split usr is dumb.

Read: http://rusty.ozlabs.org/?p=236
 ("Why Everyone Must Oppose The Merging of /usr and /")

(I managed to skip the pulseaudio implamentation mess because I
had a fancy emu10k1 SoundBlaster Live! 5.1 which does its own
hardware mixing.)

v3: revert 99f861310d3f05f4
---
 src/timedate/timedated.c | 21 +
 1 file changed, 21 insertions(+)

diff --git a/src/timedate/timedated.c b/src/timedate/timedated.c
index 753c3d1..3fbc24e 100644
--- a/src/timedate/timedated.c
+++ b/src/timedate/timedated.c
@@ -29,6 +29,7 @@
 #include "sd-bus.h"
 
 #include "util.h"
+#include "copy.h"
 #include "strv.h"
 #include "def.h"
 #include "clock-util.h"
@@ -95,6 +96,14 @@ static int context_read_data(Context *c) {
 }
 }
 
+#ifdef HAVE_SPLIT_USR
+r = read_one_line_file("/etc/timezone", c->zone);
+if (r < 0) {
+if (r != -ENOENT)
+log_warning("Failed to read /etc/timezone: %s", 
strerror(-r));
+}
+#endif
+
 have_timezone:
 if (isempty(c->zone)) {
 free(c->zone);
@@ -123,9 +132,21 @@ static int context_write_data_timezone(Context *c) {
 if (!p)
 return log_oom();
 
+#ifdef HAVE_SPLIT_USR
+r = write_string_file_atomic("/etc/timezone", c->zone);
+if (r < 0)
+return r;
+
+   /* "/usr/sha..." */
+r = copy_file((p + 2), "/etc/localtime", O_TRUNC,
+S_IRUSR|S_IWUSR|S_IRGRP|S_IROTH); /*644*/
+if (r < 0)
+return r;
+#else
 r = symlink_atomic(p, "/etc/localtime");
 if (r < 0)
 return r;
+#endif
 
 return 0;
 }
-- 
2.1.4

___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [PATCH] timedated: support split usr v3

[Shawn Landden]

On Sun, Jan 18, 2015 at 12:52 PM, Kay Sievers  wrote:

> On Sun, Jan 18, 2015 at 7:53 PM, Shawn Landden 
> wrote:
> > From: Shawn Paul Landden 
> >
> > The current Debian solution to this is really ugly, and I would rather
> > have them use the correct patch even if split usr is dumb.
>
> Please keep this local to the distro, this is no upstream material.
> /etc/timezon is redundant information in a separate file. We do not
> want to support this thing in systemd.
>
> The split /usr support is pretty much limited to locations of files,
> but should not change fundamental logic or introduce new concepts or
> config files.
>
> While I have no problem with this position as I support unified usr,
realize that without this timedated does not support split usr, and systemd
be clear about this in the "split-usr-is-broken" warning/essay.

> Thanks,
> Kay
> ___
> systemd-devel mailing list
> systemd-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/systemd-devel
>



-- 
Shawn Landden
From 0f0d2f6b3e295c8d67a689e48b23188a027ce6b3 Mon Sep 17 00:00:00 2001
From: Shawn Paul Landden 
Date: Sun, 21 Dec 2014 23:00:01 -0800
Subject: [PATCH] timedated: support split usr v3

The current Debian solution to this is really ugly, and I would rather
have them use the correct patch even if split usr is dumb.

Read: http://rusty.ozlabs.org/?p=236
 ("Why Everyone Must Oppose The Merging of /usr and /")

(I managed to skip the pulseaudio implamentation mess because I
had a fancy emu10k1 SoundBlaster Live! 5.1 which does its own
hardware mixing.)

Putting the reading of /etc/timezone inside #ifdef CONFIG_SPLIT_USR
assumes a system with never go from NORMAL_USR to SPLIT_USR

v3: revert 99f861310d3f05f4
v4: was missing a git add
---
 src/timedate/timedated.c | 21 +
 1 file changed, 21 insertions(+)

diff --git a/src/timedate/timedated.c b/src/timedate/timedated.c
index 753c3d1..7f748df 100644
--- a/src/timedate/timedated.c
+++ b/src/timedate/timedated.c
@@ -29,6 +29,7 @@
 #include "sd-bus.h"
 
 #include "util.h"
+#include "copy.h"
 #include "strv.h"
 #include "def.h"
 #include "clock-util.h"
@@ -95,6 +96,14 @@ static int context_read_data(Context *c) {
 }
 }
 
+#ifdef HAVE_SPLIT_USR
+r = read_one_line_file("/etc/timezone", &c->zone);
+if (r < 0) {
+if (r != -ENOENT)
+log_warning("Failed to read /etc/timezone: %s", strerror(-r));
+}
+#endif
+
 have_timezone:
 if (isempty(c->zone)) {
 free(c->zone);
@@ -123,9 +132,21 @@ static int context_write_data_timezone(Context *c) {
 if (!p)
 return log_oom();
 
+#ifdef HAVE_SPLIT_USR
+r = write_string_file_atomic("/etc/timezone", c->zone);
+if (r < 0)
+return r;
+
+   /* "/usr/sha..." */
+r = copy_file((p + 2), "/etc/localtime", O_TRUNC,
+S_IRUSR|S_IWUSR|S_IRGRP|S_IROTH, 0); /*644*/
+if (r < 0)
+return r;
+#else
 r = symlink_atomic(p, "/etc/localtime");
 if (r < 0)
 return r;
+#endif
 
 return 0;
 }
-- 
2.1.4

___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

[systemd-devel] [PATCH] shutdown: avoid calling `kexec` binary unnessecarily

[Shawn Landden]

Still use helper when Xen Dom0, to avoid duplicating some hairy code.
So we don't have any logic to load kexec kernels?
---
 TODO|  3 ---
 src/core/shutdown.c | 33 -
 2 files changed, 20 insertions(+), 16 deletions(-)

diff --git a/TODO b/TODO
index 883b994..68b0af6 100644
--- a/TODO
+++ b/TODO
@@ -85,9 +85,6 @@ Features:
 * maybe introduce WantsMountsFor=? Usecase:
   http://lists.freedesktop.org/archives/systemd-devel/2015-January/027729.html
 
-* rework kexec logic to use new kexec_file_load() syscall, so that we
-  don't have to call kexec tool anymore.
-
 * The udev blkid built-in should expose a property that reflects
   whether media was sensed in USB CF/SD card readers. This should then
   be used to control SYSTEMD_READY=1/0 so that USB card readers aren't
diff --git a/src/core/shutdown.c b/src/core/shutdown.c
index 71f001a..c64c05d 100644
--- a/src/core/shutdown.c
+++ b/src/core/shutdown.c
@@ -350,26 +350,33 @@ int main(int argc, char *argv[]) {
 case LINUX_REBOOT_CMD_KEXEC:
 
 if (!in_container) {
-/* We cheat and exec kexec to avoid doing all its work 
*/
-pid_t pid;
+_cleanup_free_ char *param = NULL;
 
 log_info("Rebooting with kexec.");
 
-pid = fork();
-if (pid < 0)
-log_error_errno(errno, "Failed to fork: %m");
-else if (pid == 0) {
+/* kexec-tools has a bunch of special code to make Xen 
Dom0 work,
+ * otherwise it is only doing stuff we have already 
done */
+if (access("/proc/xen", F_OK) == 0) {
+pid_t pid;
+
+pid = fork();
+if (pid < 0)
+log_error_errno(errno, "Failed to 
fork: %m");
+else if (pid == 0) {
+
+const char * const args[] = {
+KEXEC, "-e", NULL
+};
 
-const char * const args[] = {
-KEXEC, "-e", NULL
-};
+/* Child */
 
-/* Child */
+execv(args[0], (char * const *) args);
+_exit(EXIT_FAILURE);
+} else
+wait_for_terminate_and_warn("kexec", 
pid, true);
 
-execv(args[0], (char * const *) args);
-_exit(EXIT_FAILURE);
 } else
-wait_for_terminate_and_warn("kexec", pid, 
true);
+reboot(cmd);
 }
 
 cmd = RB_AUTOBOOT;
-- 
2.2.1.209.g41e5f3a

___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

[systemd-devel] [PATCH] timedated: when performing "SetTime" compensate for program lag

[Shawn Landden]

---
 TODO |  2 --
 src/timedate/timedated.c | 14 ++
 2 files changed, 14 insertions(+), 2 deletions(-)

diff --git a/TODO b/TODO
index 68b0af6..7b93404 100644
--- a/TODO
+++ b/TODO
@@ -190,8 +190,6 @@ Features:
 * we should try harder to collapse start jobs for swaps that end up being the 
same:
   http://lists.freedesktop.org/archives/systemd-devel/2014-November/025359.html
 
-* timedated should compensate on SetTime for the time spent in polkit
-
 * figure out when we can use the coarse timers
 
 * sd-resolve: drop res_query wrapping, people should call via the bus to 
resolved instead
diff --git a/src/timedate/timedated.c b/src/timedate/timedated.c
index 753c3d1..7948bfa 100644
--- a/src/timedate/timedated.c
+++ b/src/timedate/timedated.c
@@ -529,6 +529,7 @@ static int method_set_time(sd_bus *bus, sd_bus_message *m, 
void *userdata, sd_bu
 Context *c = userdata;
 int64_t utc;
 struct timespec ts;
+usec_t start, ready;
 struct tm* tm;
 int r;
 
@@ -569,6 +570,19 @@ static int method_set_time(sd_bus *bus, sd_bus_message *m, 
void *userdata, sd_bu
 if (r == 0)
 return 1;
 
+/* adjust ts for time spent in program */
+r = sd_bus_message_get_monotonic_usec(m, &start);
+if (r < 0) {
+/* we only get this data if we are using kdbus */
+if (r == -ENODATA)
+goto nodata;
+
+return r;
+}
+ready = now(CLOCK_MONOTONIC);
+timespec_store(&ts, timespec_load(&ts) + (ready - start));
+nodata:
+
 /* Set system clock */
 if (clock_settime(CLOCK_REALTIME, &ts) < 0) {
 log_error_errno(errno, "Failed to set local time: %m");
-- 
2.2.1.209.g41e5f3a

___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

[systemd-devel] [PATCH] timedated: when performing "SetTime" compensate for program lag

[Shawn Landden]

The start time could be moved back a little bit by using kdbus timestamps,
but I'm guessing that that amount of time is insignificant.
---
 TODO | 2 --
 src/timedate/timedated.c | 7 +++
 2 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/TODO b/TODO
index 93dfa60..88055d3 100644
--- a/TODO
+++ b/TODO
@@ -190,8 +190,6 @@ Features:
 * we should try harder to collapse start jobs for swaps that end up being the 
same:
   http://lists.freedesktop.org/archives/systemd-devel/2014-November/025359.html
 
-* timedated should compensate on SetTime for the time spent in polkit
-
 * figure out when we can use the coarse timers
 
 * sd-resolve: drop res_query wrapping, people should call via the bus to 
resolved instead
diff --git a/src/timedate/timedated.c b/src/timedate/timedated.c
index 753c3d1..603e155 100644
--- a/src/timedate/timedated.c
+++ b/src/timedate/timedated.c
@@ -529,6 +529,7 @@ static int method_set_time(sd_bus *bus, sd_bus_message *m, 
void *userdata, sd_bu
 Context *c = userdata;
 int64_t utc;
 struct timespec ts;
+usec_t start, ready;
 struct tm* tm;
 int r;
 
@@ -536,6 +537,8 @@ static int method_set_time(sd_bus *bus, sd_bus_message *m, 
void *userdata, sd_bu
 assert(m);
 assert(c);
 
+start = now(CLOCK_MONOTONIC);
+
 if (c->use_ntp)
 return sd_bus_error_setf(error, 
BUS_ERROR_AUTOMATIC_TIME_SYNC_ENABLED, "Automatic time synchronization is 
enabled");
 
@@ -569,6 +572,10 @@ static int method_set_time(sd_bus *bus, sd_bus_message *m, 
void *userdata, sd_bu
 if (r == 0)
 return 1;
 
+/* adjust ts for time spent in polkit */
+ready = now(CLOCK_MONOTONIC);
+timespec_store(&ts, timespec_load(&ts) + (ready - start));
+
 /* Set system clock */
 if (clock_settime(CLOCK_REALTIME, &ts) < 0) {
 log_error_errno(errno, "Failed to set local time: %m");
-- 
2.2.1.209.g41e5f3a

___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

[systemd-devel] [PATCH] timedated: when performing "SetTime" compensate for program lag

[Shawn Landden]

I can't test this as kdbus doesn't build against Linux 3.19
---
 TODO |  2 --
 src/timedate/timedated.c | 14 ++
 2 files changed, 14 insertions(+), 2 deletions(-)

diff --git a/TODO b/TODO
index 93dfa60..88055d3 100644
--- a/TODO
+++ b/TODO
@@ -190,8 +190,6 @@ Features:
 * we should try harder to collapse start jobs for swaps that end up being the 
same:
   http://lists.freedesktop.org/archives/systemd-devel/2014-November/025359.html
 
-* timedated should compensate on SetTime for the time spent in polkit
-
 * figure out when we can use the coarse timers
 
 * sd-resolve: drop res_query wrapping, people should call via the bus to 
resolved instead
diff --git a/src/timedate/timedated.c b/src/timedate/timedated.c
index 753c3d1..1287038 100644
--- a/src/timedate/timedated.c
+++ b/src/timedate/timedated.c
@@ -529,6 +529,7 @@ static int method_set_time(sd_bus *bus, sd_bus_message *m, 
void *userdata, sd_bu
 Context *c = userdata;
 int64_t utc;
 struct timespec ts;
+usec_t start, ready;
 struct tm* tm;
 int r;
 
@@ -569,6 +570,17 @@ static int method_set_time(sd_bus *bus, sd_bus_message *m, 
void *userdata, sd_bu
 if (r == 0)
 return 1;
 
+/* adjust ts for time spent in program */
+r = sd_bus_message_get_monotonic_usec(m, &start);
+if (r == -ENODATA)
+continue;
+else if (r < 0)
+return r;
+else {
+ready = now(CLOCK_MONOTONIC);
+timespec_store(&ts, timespec_load(&ts) + (ready - start));
+}
+
 /* Set system clock */
 if (clock_settime(CLOCK_REALTIME, &ts) < 0) {
 log_error_errno(errno, "Failed to set local time: %m");
@@ -702,6 +714,8 @@ int main(int argc, char *argv[]) {
 if (r < 0)
 goto finish;
 
+(void)sd_bus_negotiate_timestamp(bus, true);
+
 r = context_read_data(&context);
 if (r < 0) {
 log_error_errno(r, "Failed to read time zone data: %m");
-- 
2.2.1.209.g41e5f3a

___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

[systemd-devel] [PATCH] configure: turn off -Wlogical-not-parentheses

[Shawn Landden]

Introduced in gcc-5

These errors are really annoying. I can get behind clarification of nested ifs,
but this is overkill.
---
 configure.ac | 1 +
 1 file changed, 1 insertion(+)

diff --git a/configure.ac b/configure.ac
index 97a29d6..e646db7 100644
--- a/configure.ac
+++ b/configure.ac
@@ -187,6 +187,7 @@ CC_CHECK_FLAGS_APPEND([with_cflags], [CFLAGS], [\
 -Wno-unused-parameter \
 -Wno-missing-field-initializers \
 -Wno-unused-result \
+-Wno-logical-not-parentheses \
 -Werror=overflow \
 -Wdate-time \
 -Wnested-externs \
-- 
2.2.1.209.g41e5f3a

___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [PATCH] configure: turn off -Wlogical-not-parentheses

[Shawn Landden]

On Mon, Feb 16, 2015 at 2:03 PM, Zbigniew Jędrzejewski-Szmek <
zbys...@in.waw.pl> wrote:

> On Mon, Feb 16, 2015 at 02:02:17PM -0800, Shawn Landden wrote:
> > Introduced in gcc-5
> >
> > These errors are really annoying. I can get behind clarification of
> nested ifs,
> > but this is overkill.
> > ---
> >  configure.ac | 1 +
> >  1 file changed, 1 insertion(+)
> >
> > diff --git a/configure.ac b/configure.ac
> > index 97a29d6..e646db7 100644
> > --- a/configure.ac
> > +++ b/configure.ac
> > @@ -187,6 +187,7 @@ CC_CHECK_FLAGS_APPEND([with_cflags], [CFLAGS], [\
> >  -Wno-unused-parameter \
> >  -Wno-missing-field-initializers \
> >  -Wno-unused-result \
> > +-Wno-logical-not-parentheses \
> >  -Werror=overflow \
> >  -Wdate-time \
> >  -Wnested-externs \
> This does not work, gcc does not allow testing for -Wno- options.
>

That is a good thing as otherwise it would break older compilers. This
fixes the warnings for me.

>
> Zbyszek
> _______
> systemd-devel mailing list
> systemd-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/systemd-devel
>



-- 
Shawn Landden
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [PATCH] shutdown: avoid calling `kexec` binary unnessecarily

[Shawn Landden]

On Mon, Feb 16, 2015 at 5:08 AM, Lennart Poettering 
wrote:

> On Fri, 13.02.15 14:18, Shawn Landden (sh...@churchofgit.com) wrote:
>
> > Still use helper when Xen Dom0, to avoid duplicating some hairy
> > code.
>
> Hmm, what precisely does the helper do on xen?
>
> > So we don't have any logic to load kexec kernels?
>
> Currently we don't.
>
> My hope though was that we can make the whole kexec thing work without
> having kexec-tools installed. With the new kernel syscall for loading
> the kernel we should be able to implement this all without any other
> tools.
>
> Ideally we'd not use any external tools anymore, not even in the Xen
> case, hence I am curious what precisely the special hookup for Xen is
> here...?
>
> Lennart
>
>
https://git.kernel.org/cgit/utils/kernel/kexec/kexec-tools.git/tree/kexec/kexec-xen.c

I've attached my patch.
I'm having a problem with kexec_file_load() returning ENOMEM that I havn't
resolved.

> --
> Lennart Poettering, Red Hat
> ___
> systemd-devel mailing list
> systemd-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/systemd-devel
>



-- 
Shawn Landden
From d5446e324143f55e67bc2defe1c78a4ea4201142 Mon Sep 17 00:00:00 2001
From: Shawn Landden 
Date: Fri, 13 Feb 2015 13:48:07 -0800
Subject: [PATCH] shutdown: avoid calling `kexec` binary unnessecarily

Still use helper when Xen Dom0, to avoid duplicating some hairy code.
So we don't have any logic to load kexec kernels?
---
 TODO |   3 --
 src/core/shutdown.c  | 121 ++-
 src/shared/missing.h |   7 +++
 3 files changed, 116 insertions(+), 15 deletions(-)

diff --git a/TODO b/TODO
index 255a4f2..d914d2c 100644
--- a/TODO
+++ b/TODO
@@ -90,9 +90,6 @@ Features:
 * maybe introduce WantsMountsFor=? Usecase:
   http://lists.freedesktop.org/archives/systemd-devel/2015-January/027729.html
 
-* rework kexec logic to use new kexec_file_load() syscall, so that we
-  don't have to call kexec tool anymore.
-
 * The udev blkid built-in should expose a property that reflects
   whether media was sensed in USB CF/SD card readers. This should then
   be used to control SYSTEMD_READY=1/0 so that USB card readers aren't
diff --git a/src/core/shutdown.c b/src/core/shutdown.c
index 71f001a..14febf3 100644
--- a/src/core/shutdown.c
+++ b/src/core/shutdown.c
@@ -19,6 +19,7 @@
   along with systemd; If not, see <http://www.gnu.org/licenses/>.
 ***/
 
+#include 
 #include 
 #include 
 #include 
@@ -27,6 +28,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -138,6 +140,35 @@ static int parse_argv(int argc, char *argv[]) {
 return 0;
 }
 
+/*
+ * Remove parameter from a kernel command line. Helper function for kexec.
+ * (copied from kexec-tools)
+ */
+static void remove_parameter(char *line, const char *param_name) {
+char *start, *end;
+
+start = strstr(line, param_name);
+
+/* parameter not found */
+if (!start)
+return;
+
+/*
+ * check if that's really the start of a parameter and not in
+ * the middle of the word
+ */
+if (start != line && !isspace(*(start-1)))
+return;
+
+end = strstr(start, " ");
+if (!end)
+*start = 0;
+else {
+memmove(start, end+1, strlen(end));
+*(end + strlen(end)) = 0;
+}
+}
+
 static int switch_root_initramfs(void) {
 if (mount("/run/initramfs", "/run/initramfs", NULL, MS_BIND, NULL) < 0)
 return log_error_errno(errno, "Failed to mount bind /run/initramfs on /run/initramfs: %m");
@@ -152,6 +183,57 @@ static int switch_root_initramfs(void) {
 return switch_root("/run/initramfs", "/oldroot", false, MS_BIND);
 }
 
+static int kernel_load(bool overwrite) {
+int r = -ENOTSUP;
+
+/* only x86_64 and x32 in 3.18 */
+#ifdef __NR_kexec_file_load
+/* If uses has no already loaded a kexec kernel assume they
+ * want the same one they are currently running. */
+if (!overwrite && !kexec_loaded()) {
+struct utsname u;
+_cleanup_free_ char *vmlinuz = NULL, *initrd = NULL, *cmdline = NULL;
+_cleanup_close_ int vmlinuz_fd = -1, initrd_fd = -1;
+
+r = uname(&u);
+if (r < 0)
+return -errno;
+
+vmlinuz = malloc(strlen("/boot/vmlinuz-") + strlen(u.release) + 1);
+initrd  = malloc(strlen("/boot/initrd.img-") + strlen(u.release)

Re: [systemd-devel] [PATCH] shutdown: avoid calling `kexec` binary unnessecarily

[Shawn Landden]

On Tue, Feb 17, 2015 at 7:00 AM, Zbigniew Jędrzejewski-Szmek <
zbys...@in.waw.pl> wrote:

> On Mon, Feb 16, 2015 at 07:53:47PM -0800, Shawn Landden wrote:
> > On Mon, Feb 16, 2015 at 5:08 AM, Lennart Poettering <
> lenn...@poettering.net>
> > wrote:
> >
> > > On Fri, 13.02.15 14:18, Shawn Landden (sh...@churchofgit.com) wrote:
> > >
> > > > Still use helper when Xen Dom0, to avoid duplicating some hairy
> > > > code.
> > >
> > > Hmm, what precisely does the helper do on xen?
> > >
> > > > So we don't have any logic to load kexec kernels?
> > >
> > > Currently we don't.
> > >
> > > My hope though was that we can make the whole kexec thing work without
> > > having kexec-tools installed. With the new kernel syscall for loading
> > > the kernel we should be able to implement this all without any other
> > > tools.
> > >
> > > Ideally we'd not use any external tools anymore, not even in the Xen
> > > case, hence I am curious what precisely the special hookup for Xen is
> > > here...?
> > >
> > > Lennart
> > >
> > >
> >
> https://git.kernel.org/cgit/utils/kernel/kexec/kexec-tools.git/tree/kexec/kexec-xen.c
> >
> > I've attached my patch.
> > I'm having a problem with kexec_file_load() returning ENOMEM that I
> havn't
> > resolved.
> >
> > > --
> > > Lennart Poettering, Red Hat
> > > ___
> > > systemd-devel mailing list
> > > systemd-devel@lists.freedesktop.org
> > > http://lists.freedesktop.org/mailman/listinfo/systemd-devel
> > >
> >
> >
> >
> > --
> > Shawn Landden
>
> > From d5446e324143f55e67bc2defe1c78a4ea4201142 Mon Sep 17 00:00:00 2001
> > From: Shawn Landden 
> > Date: Fri, 13 Feb 2015 13:48:07 -0800
> > Subject: [PATCH] shutdown: avoid calling `kexec` binary unnessecarily
> >
> > Still use helper when Xen Dom0, to avoid duplicating some hairy code.
> > So we don't have any logic to load kexec kernels?
> > ---
> >  TODO |   3 --
> >  src/core/shutdown.c  | 121
> ++-
> >  src/shared/missing.h |   7 +++
> >  3 files changed, 116 insertions(+), 15 deletions(-)
> >
> > diff --git a/TODO b/TODO
> > index 255a4f2..d914d2c 100644
> > --- a/TODO
> > +++ b/TODO
> > @@ -90,9 +90,6 @@ Features:
> >  * maybe introduce WantsMountsFor=? Usecase:
> >
> http://lists.freedesktop.org/archives/systemd-devel/2015-January/027729.html
> >
> > -* rework kexec logic to use new kexec_file_load() syscall, so that we
> > -  don't have to call kexec tool anymore.
> > -
> >  * The udev blkid built-in should expose a property that reflects
> >whether media was sensed in USB CF/SD card readers. This should then
> >be used to control SYSTEMD_READY=1/0 so that USB card readers aren't
> > diff --git a/src/core/shutdown.c b/src/core/shutdown.c
> > index 71f001a..14febf3 100644
> > --- a/src/core/shutdown.c
> > +++ b/src/core/shutdown.c
> > @@ -19,6 +19,7 @@
> >along with systemd; If not, see <http://www.gnu.org/licenses/>.
> >  ***/
> >
> > +#include 
> >  #include 
> >  #include 
> >  #include 
> > @@ -27,6 +28,7 @@
> >  #include 
> >  #include 
> >  #include 
> > +#include 
> >  #include 
> >  #include 
> >  #include 
> > @@ -138,6 +140,35 @@ static int parse_argv(int argc, char *argv[]) {
> >  return 0;
> >  }
> >
> > +/*
> > + * Remove parameter from a kernel command line. Helper function for
> kexec.
> > + * (copied from kexec-tools)
> > + */
> > +static void remove_parameter(char *line, const char *param_name) {
> > +char *start, *end;
> > +
> > +start = strstr(line, param_name);
> > +
> > +/* parameter not found */
> > +if (!start)
> > +return;
> > +
> > +/*
> > + * check if that's really the start of a parameter and not in
> > + * the middle of the word
> > + */
> > +if (start != line && !isspace(*(start-1)))
> > +return;
> > +
> > +end = strstr(start, " ");
> > +if (!end)
> > +*start = 0;
> > +else {
> > +me

Re: [systemd-devel] [PATCH] shutdown: avoid calling `kexec` binary unnessecarily

[Shawn Landden]

On Tue, Feb 17, 2015 at 4:34 PM, Shawn Landden 
wrote:

> On Tue, Feb 17, 2015 at 7:00 AM, Zbigniew Jędrzejewski-Szmek <
> zbys...@in.waw.pl> wrote:
>
>> On Mon, Feb 16, 2015 at 07:53:47PM -0800, Shawn Landden wrote:
>> > On Mon, Feb 16, 2015 at 5:08 AM, Lennart Poettering <
>> lenn...@poettering.net>
>> > wrote:
>> >
>> > > On Fri, 13.02.15 14:18, Shawn Landden (sh...@churchofgit.com) wrote:
>> > >
>> > > > Still use helper when Xen Dom0, to avoid duplicating some hairy
>> > > > code.
>> > >
>> > > Hmm, what precisely does the helper do on xen?
>> > >
>> > > > So we don't have any logic to load kexec kernels?
>> > >
>> > > Currently we don't.
>> > >
>> > > My hope though was that we can make the whole kexec thing work without
>> > > having kexec-tools installed. With the new kernel syscall for loading
>> > > the kernel we should be able to implement this all without any other
>> > > tools.
>> > >
>> > > Ideally we'd not use any external tools anymore, not even in the Xen
>> > > case, hence I am curious what precisely the special hookup for Xen is
>> > > here...?
>> > >
>> > > Lennart
>> > >
>> > >
>> >
>> https://git.kernel.org/cgit/utils/kernel/kexec/kexec-tools.git/tree/kexec/kexec-xen.c
>> >
>> > I've attached my patch.
>> > I'm having a problem with kexec_file_load() returning ENOMEM that I
>> havn't
>> > resolved.
>> >
>> > > --
>> > > Lennart Poettering, Red Hat
>> > > ___
>> > > systemd-devel mailing list
>> > > systemd-devel@lists.freedesktop.org
>> > > http://lists.freedesktop.org/mailman/listinfo/systemd-devel
>> > >
>> >
>> >
>> >
>> > --
>> > Shawn Landden
>>
>> > From d5446e324143f55e67bc2defe1c78a4ea4201142 Mon Sep 17 00:00:00 2001
>> > From: Shawn Landden 
>> > Date: Fri, 13 Feb 2015 13:48:07 -0800
>> > Subject: [PATCH] shutdown: avoid calling `kexec` binary unnessecarily
>> >
>> > Still use helper when Xen Dom0, to avoid duplicating some hairy code.
>> > So we don't have any logic to load kexec kernels?
>> > ---
>> >  TODO |   3 --
>> >  src/core/shutdown.c  | 121
>> ++-
>> >  src/shared/missing.h |   7 +++
>> >  3 files changed, 116 insertions(+), 15 deletions(-)
>> >
>> > diff --git a/TODO b/TODO
>> > index 255a4f2..d914d2c 100644
>> > --- a/TODO
>> > +++ b/TODO
>> > @@ -90,9 +90,6 @@ Features:
>> >  * maybe introduce WantsMountsFor=? Usecase:
>> >
>> http://lists.freedesktop.org/archives/systemd-devel/2015-January/027729.html
>> >
>> > -* rework kexec logic to use new kexec_file_load() syscall, so that we
>> > -  don't have to call kexec tool anymore.
>> > -
>> >  * The udev blkid built-in should expose a property that reflects
>> >whether media was sensed in USB CF/SD card readers. This should then
>> >be used to control SYSTEMD_READY=1/0 so that USB card readers aren't
>> > diff --git a/src/core/shutdown.c b/src/core/shutdown.c
>> > index 71f001a..14febf3 100644
>> > --- a/src/core/shutdown.c
>> > +++ b/src/core/shutdown.c
>> > @@ -19,6 +19,7 @@
>> >along with systemd; If not, see <http://www.gnu.org/licenses/>.
>> >  ***/
>> >
>> > +#include 
>> >  #include 
>> >  #include 
>> >  #include 
>> > @@ -27,6 +28,7 @@
>> >  #include 
>> >  #include 
>> >  #include 
>> > +#include 
>> >  #include 
>> >  #include 
>> >  #include 
>> > @@ -138,6 +140,35 @@ static int parse_argv(int argc, char *argv[]) {
>> >  return 0;
>> >  }
>> >
>> > +/*
>> > + * Remove parameter from a kernel command line. Helper function for
>> kexec.
>> > + * (copied from kexec-tools)
>> > + */
>> > +static void remove_parameter(char *line, const char *param_name) {
>> > +char *start, *end;
>> > +
>> > +start = strstr(line, param_name);
>> > +
>> > +/* parameter not found */
>> > +if (!start)
>> > +  

Re: [systemd-devel] [PATCH] shutdown: avoid calling `kexec` binary unnessecarily

[Shawn Landden]

The bootloader spec does not say which entry is to be the default. I cannot
support the spec unless I can choose a single default kernel.

http://freedesktop.org/wiki/Specifications/BootLoaderSpec/

On Tue, Feb 17, 2015 at 7:00 AM, Zbigniew Jędrzejewski-Szmek <
zbys...@in.waw.pl> wrote:

> On Mon, Feb 16, 2015 at 07:53:47PM -0800, Shawn Landden wrote:
> > On Mon, Feb 16, 2015 at 5:08 AM, Lennart Poettering <
> lenn...@poettering.net>
> > wrote:
> >
> > > On Fri, 13.02.15 14:18, Shawn Landden (sh...@churchofgit.com) wrote:
> > >
> > > > Still use helper when Xen Dom0, to avoid duplicating some hairy
> > > > code.
> > >
> > > Hmm, what precisely does the helper do on xen?
> > >
> > > > So we don't have any logic to load kexec kernels?
> > >
> > > Currently we don't.
> > >
> > > My hope though was that we can make the whole kexec thing work without
> > > having kexec-tools installed. With the new kernel syscall for loading
> > > the kernel we should be able to implement this all without any other
> > > tools.
> > >
> > > Ideally we'd not use any external tools anymore, not even in the Xen
> > > case, hence I am curious what precisely the special hookup for Xen is
> > > here...?
> > >
> > > Lennart
> > >
> > >
> >
> https://git.kernel.org/cgit/utils/kernel/kexec/kexec-tools.git/tree/kexec/kexec-xen.c
> >
> > I've attached my patch.
> > I'm having a problem with kexec_file_load() returning ENOMEM that I
> havn't
> > resolved.
> >
> > > --
> > > Lennart Poettering, Red Hat
> > > ___
> > > systemd-devel mailing list
> > > systemd-devel@lists.freedesktop.org
> > > http://lists.freedesktop.org/mailman/listinfo/systemd-devel
> > >
> >
> >
> >
> > --
> > Shawn Landden
>
> > From d5446e324143f55e67bc2defe1c78a4ea4201142 Mon Sep 17 00:00:00 2001
> > From: Shawn Landden 
> > Date: Fri, 13 Feb 2015 13:48:07 -0800
> > Subject: [PATCH] shutdown: avoid calling `kexec` binary unnessecarily
> >
> > Still use helper when Xen Dom0, to avoid duplicating some hairy code.
> > So we don't have any logic to load kexec kernels?
> > ---
> >  TODO |   3 --
> >  src/core/shutdown.c  | 121
> ++-
> >  src/shared/missing.h |   7 +++
> >  3 files changed, 116 insertions(+), 15 deletions(-)
> >
> > diff --git a/TODO b/TODO
> > index 255a4f2..d914d2c 100644
> > --- a/TODO
> > +++ b/TODO
> > @@ -90,9 +90,6 @@ Features:
> >  * maybe introduce WantsMountsFor=? Usecase:
> >
> http://lists.freedesktop.org/archives/systemd-devel/2015-January/027729.html
> >
> > -* rework kexec logic to use new kexec_file_load() syscall, so that we
> > -  don't have to call kexec tool anymore.
> > -
> >  * The udev blkid built-in should expose a property that reflects
> >whether media was sensed in USB CF/SD card readers. This should then
> >be used to control SYSTEMD_READY=1/0 so that USB card readers aren't
> > diff --git a/src/core/shutdown.c b/src/core/shutdown.c
> > index 71f001a..14febf3 100644
> > --- a/src/core/shutdown.c
> > +++ b/src/core/shutdown.c
> > @@ -19,6 +19,7 @@
> >along with systemd; If not, see <http://www.gnu.org/licenses/>.
> >  ***/
> >
> > +#include 
> >  #include 
> >  #include 
> >  #include 
> > @@ -27,6 +28,7 @@
> >  #include 
> >  #include 
> >  #include 
> > +#include 
> >  #include 
> >  #include 
> >  #include 
> > @@ -138,6 +140,35 @@ static int parse_argv(int argc, char *argv[]) {
> >  return 0;
> >  }
> >
> > +/*
> > + * Remove parameter from a kernel command line. Helper function for
> kexec.
> > + * (copied from kexec-tools)
> > + */
> > +static void remove_parameter(char *line, const char *param_name) {
> > +char *start, *end;
> > +
> > +start = strstr(line, param_name);
> > +
> > +/* parameter not found */
> > +if (!start)
> > +return;
> > +
> > +/*
> > + * check if that's really the start of a parameter and not in
> > + * the middle of the word
> > + */
> > +if (start != line && !isspace(*(start-1)))
> > +return;
> > +
&

[systemd-devel] [PATCH] fix build against v3.20-rc1

[Shawn Landden]

---
 fs.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/fs.c b/fs.c
index 22ca62b..f751392 100644
--- a/fs.c
+++ b/fs.c
@@ -208,7 +208,6 @@ static struct inode *fs_inode_get(struct super_block *sb,
 
inode->i_private = kdbus_node_ref(node);
inode->i_mapping->a_ops = &empty_aops;
-   inode->i_mapping->backing_dev_info = &noop_backing_dev_info;
inode->i_mode = node->mode & S_IALLUGO;
inode->i_atime = inode->i_ctime = inode->i_mtime = CURRENT_TIME;
inode->i_uid = node->uid;
-- 
2.2.1.209.g41e5f3a

___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

[systemd-devel] [PATCH 3/8] power: refactor the three power management binaries to src/power

[Shawn Landden]

---
 Makefile.am   |   6 +-
 src/core/shutdown.c   | 420 -
 src/power/Makefile|  28 +++
 src/power/shutdown.c  | 420 +
 src/power/shutdownd.c | 461 ++
 src/power/sleep.c | 219 ++
 src/shutdownd/Makefile|   1 -
 src/shutdownd/shutdownd.c | 461 --
 src/sleep/Makefile|   1 -
 src/sleep/sleep.c | 219 --
 10 files changed, 1131 insertions(+), 1105 deletions(-)
 delete mode 100644 src/core/shutdown.c
 create mode 100644 src/power/Makefile
 create mode 100644 src/power/shutdown.c
 create mode 100644 src/power/shutdownd.c
 create mode 100644 src/power/sleep.c
 delete mode 12 src/shutdownd/Makefile
 delete mode 100644 src/shutdownd/shutdownd.c
 delete mode 12 src/sleep/Makefile
 delete mode 100644 src/sleep/sleep.c

diff --git a/Makefile.am b/Makefile.am
index ba63f68..52ec7ef 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -2122,7 +2122,7 @@ systemd_update_done_LDADD = \
 
 # 
--
 systemd_shutdownd_SOURCES = \
-   src/shutdownd/shutdownd.c
+   src/power/shutdownd.c
 
 systemd_shutdownd_LDADD = \
libsystemd-label.la \
@@ -2136,7 +2136,7 @@ dist_doc_DATA += \
 systemd_shutdown_SOURCES = \
src/core/umount.c \
src/core/umount.h \
-   src/core/shutdown.c \
+   src/power/shutdown.c \
src/core/mount-setup.c \
src/core/mount-setup.h \
src/core/killall.h \
@@ -2340,7 +2340,7 @@ systemd_sysctl_LDADD = \
 
 # 
--
 systemd_sleep_SOURCES = \
-   src/sleep/sleep.c
+   src/power/sleep.c
 
 systemd_sleep_LDADD = \
libsystemd-shared.la
diff --git a/src/core/shutdown.c b/src/core/shutdown.c
deleted file mode 100644
index 71f001a..000
--- a/src/core/shutdown.c
+++ /dev/null
@@ -1,420 +0,0 @@
-/*-*- Mode: C; c-basic-offset: 8; indent-tabs-mode: nil -*-*/
-
-/***
-  This file is part of systemd.
-
-  Copyright 2010 ProFUSION embedded systems
-
-  systemd is free software; you can redistribute it and/or modify it
-  under the terms of the GNU Lesser General Public License as published by
-  the Free Software Foundation; either version 2.1 of the License, or
-  (at your option) any later version.
-
-  systemd is distributed in the hope that it will be useful, but
-  WITHOUT ANY WARRANTY; without even the implied warranty of
-  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
-  Lesser General Public License for more details.
-
-  You should have received a copy of the GNU Lesser General Public License
-  along with systemd; If not, see .
-***/
-
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-
-#include "missing.h"
-#include "log.h"
-#include "fileio.h"
-#include "umount.h"
-#include "util.h"
-#include "mkdir.h"
-#include "virt.h"
-#include "watchdog.h"
-#include "killall.h"
-#include "cgroup-util.h"
-#include "def.h"
-#include "switch-root.h"
-#include "strv.h"
-
-#define FINALIZE_ATTEMPTS 50
-
-static char* arg_verb;
-
-static int parse_argv(int argc, char *argv[]) {
-enum {
-ARG_LOG_LEVEL = 0x100,
-ARG_LOG_TARGET,
-ARG_LOG_COLOR,
-ARG_LOG_LOCATION,
-};
-
-static const struct option options[] = {
-{ "log-level", required_argument, NULL, ARG_LOG_LEVEL},
-{ "log-target",required_argument, NULL, ARG_LOG_TARGET   },
-{ "log-color", optional_argument, NULL, ARG_LOG_COLOR},
-{ "log-location",  optional_argument, NULL, ARG_LOG_LOCATION },
-{}
-};
-
-int c, r;
-
-assert(argc >= 1);
-assert(argv);
-
-/* "-" prevents getopt from permuting argv[] and moving the verb away
- * from argv[1]. Our interface to initrd promises it'll be there. */
-while ((c = getopt_long(argc, argv, "-", options, NULL)) >= 0)
-switch (c) {
-
-case ARG_LOG_LEVEL:
-r = log_set_max_level_from_string(optarg);
-if (r < 0)
-log_error("Failed to parse log level %s, 
ignoring.", optarg);
-
-break;
-
-case ARG_LOG_TARGET:
-r = log_set_target_from_string(optarg);
-if (r < 0)
-log_error("Failed to parse log target %s, 
ignoring", optarg);
-
-break;
-
-case ARG_LOG_COLOR:
-
- 

[systemd-devel] [PATCH 6/8] add rbtree implamentation

[Shawn Landden]

from https://github.com/fbuihuu/libtree (LGPLv2.1+)
---
 Makefile.am |   2 +
 src/shared/rbtree.c | 482 
 src/shared/rbtree.h |  79 +
 3 files changed, 563 insertions(+)
 create mode 100644 src/shared/rbtree.c
 create mode 100644 src/shared/rbtree.h

diff --git a/Makefile.am b/Makefile.am
index 52ec7ef..e0813a6 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -902,6 +902,8 @@ libsystemd_shared_la_SOURCES = \
src/shared/verbs.h \
src/shared/sigbus.c \
src/shared/sigbus.h \
+   src/shared/rbtree.c \
+   src/shared/rbtree.h \
src/shared/build.h \
src/shared/import-util.c \
src/shared/import-util.h
diff --git a/src/shared/rbtree.c b/src/shared/rbtree.c
new file mode 100644
index 000..6b77b0f
--- /dev/null
+++ b/src/shared/rbtree.c
@@ -0,0 +1,482 @@
+/*
+ * rbtree - Implements a red-black tree with parent pointers.
+ *
+ * Copyright (C) 2010-2014 Franck Bui-Huu 
+ *
+ * This file is part of libtree which is free software; you can
+ * redistribute it and/or modify it under the terms of the GNU Lesser
+ * General Public License as published by the Free Software
+ * Foundation; either version 2.1 of the License, or (at your option)
+ * any later version.
+ *
+ * This library is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
+ *
+ * See the LICENSE file for license rights and limitations.
+ */
+
+/*
+ * For recall a red-black tree has the following properties:
+ *
+ * 1. All nodes are either BLACK or RED
+ * 2. Leafs are BLACK
+ * 3. A RED node has BLACK children only
+ * 4. Path from a node to any leafs has the same number of BLACK nodes.
+ *
+ */
+#include "rbtree.h"
+
+/*
+ * Some helpers
+ */
+#ifdef UINTPTR_MAX
+
+static inline enum rb_color get_color(const struct rbtree_node *node)
+{
+   return node->parent & 1;
+}
+
+static inline void set_color(enum rb_color color, struct rbtree_node *node)
+{
+   node->parent = (node->parent & ~1UL) | color;
+}
+
+static inline struct rbtree_node *get_parent(const struct rbtree_node *node)
+{
+   return (struct rbtree_node *)(node->parent & ~1UL);
+}
+
+static inline void set_parent(struct rbtree_node *parent, struct rbtree_node 
*node)
+{
+   node->parent = (uintptr_t)parent | (node->parent & 1);
+}
+
+#else
+
+static inline enum rb_color get_color(const struct rbtree_node *node)
+{
+   return node->color;
+}
+
+static inline void set_color(enum rb_color color, struct rbtree_node *node)
+{
+   node->color = color;
+}
+
+static inline struct rbtree_node *get_parent(const struct rbtree_node *node)
+{
+   return node->parent;
+}
+
+static inline void set_parent(struct rbtree_node *parent, struct rbtree_node 
*node)
+{
+   node->parent = parent;
+}
+
+#endif /* UINTPTR_MAX */
+
+static inline int is_root(struct rbtree_node *node)
+{
+   return get_parent(node) == NULL;
+}
+
+static inline int is_black(struct rbtree_node *node)
+{
+   return get_color(node) == RB_BLACK;
+}
+
+static inline int is_red(struct rbtree_node *node)
+{
+   return !is_black(node);
+}
+
+/*
+ * Iterators
+ */
+static inline struct rbtree_node *get_first(struct rbtree_node *node)
+{
+   while (node->left)
+   node = node->left;
+   return node;
+}
+
+static inline struct rbtree_node *get_last(struct rbtree_node *node)
+{
+   while (node->right)
+   node = node->right;
+   return node;
+}
+
+struct rbtree_node *rbtree_first(const struct rbtree *tree)
+{
+   return tree->first;
+}
+
+struct rbtree_node *rbtree_last(const struct rbtree *tree)
+{
+   return tree->last;
+}
+
+struct rbtree_node *rbtree_next(const struct rbtree_node *node)
+{
+   struct rbtree_node *parent;
+
+   if (node->right)
+   return get_first(node->right);
+
+   while ((parent = get_parent(node)) && parent->right == node)
+   node = parent;
+   return parent;
+}
+
+struct rbtree_node *rbtree_prev(const struct rbtree_node *node)
+{
+   struct rbtree_node *parent;
+
+   if (node->left)
+   return get_last(node->left);
+
+   while ((parent = get_parent(node)) && parent->left == node)
+   node = parent;
+   return parent;
+}
+
+/*
+ * 'pparent' and 'is_left' are only used for insertions. Normally GCC
+ * will notice this and get rid of them for lookups.
+ */
+static inline struct rbtree_node *do_lookup(const struct rbtree_node *key,
+   const struct rbtree *tree,
+   struct rbtree_node **pparent,
+   int *is_left)
+{
+   struct rbtree_node *node = tree->root;
+
+   *pparent = NULL;
+   *is_left = 0;
+
+   while (node) {
+   int res = tree->cmp_fn(node, key);
+   

[systemd-devel] [PATCH 5/8] power: these binaries are internal APIs

[Shawn Landden]

They are not executed by a user (they all check how they were executed)
so we can use assert() in main() just like we would anywhere else.
---
 src/power/shutdown.c  | 20 ++--
 src/power/shutdownd.c | 22 --
 src/power/sleep.c | 14 +++---
 3 files changed, 25 insertions(+), 31 deletions(-)

diff --git a/src/power/shutdown.c b/src/power/shutdown.c
index 71f001a..ffd12b8 100644
--- a/src/power/shutdown.c
+++ b/src/power/shutdown.c
@@ -162,24 +162,24 @@ int main(int argc, char *argv[]) {
 int cmd, r;
 static const char* const dirs[] = {SYSTEM_SHUTDOWN_PATH, NULL};
 
-log_parse_environment();
-r = parse_argv(argc, argv);
-if (r < 0)
-goto error;
+/* we are executed by execve() (without fork())
+ * pid 1's main() at the bottom of src/core/main.c
+ */
+assert(getpid() == 1);
 
+log_set_target(LOG_TARGET_AUTO);
+log_parse_environment();
 /* journald will die if not gone yet. The log target defaults
  * to console, but may have been changed by command line options. */
-
 log_close_console(); /* force reopen of /dev/console */
 log_open();
 
+r = parse_argv(argc, argv);
+assert(r >= 0);
+
 umask(0022);
 
-if (getpid() != 1) {
-log_error("Not executed by init (PID 1).");
-r = -EPERM;
-goto error;
-}
+in_container = detect_container(NULL) > 0;
 
 if (streq(arg_verb, "reboot"))
 cmd = RB_AUTOBOOT;
diff --git a/src/power/shutdownd.c b/src/power/shutdownd.c
index 60a6468..742e1d5 100644
--- a/src/power/shutdownd.c
+++ b/src/power/shutdownd.c
@@ -264,15 +264,9 @@ int main(int argc, char *argv[]) {
 bool exec_shutdown = false, unlink_nologin = false;
 unsigned i;
 
-if (getppid() != 1) {
-log_error("This program should be invoked by init only.");
-return EXIT_FAILURE;
-}
-
-if (argc > 1) {
-log_error("This program does not take arguments.");
-return EXIT_FAILURE;
-}
+/* we are executed through 
systemd-shutdownd.socket->systemd-shutdownd.service */
+assert(getppid() == 1);
+assert(argc == 0);
 
 log_set_target(LOG_TARGET_AUTO);
 log_parse_environment();
@@ -281,15 +275,7 @@ int main(int argc, char *argv[]) {
 umask(0022);
 
 n_fds = sd_listen_fds(true);
-if (n_fds < 0) {
-log_error_errno(r, "Failed to read listening file descriptors 
from environment: %m");
-return EXIT_FAILURE;
-}
-
-if (n_fds != 1) {
-log_error("Need exactly one file descriptor.");
-return EXIT_FAILURE;
-}
+assert(n_fds == 1);
 
 pollfd[FD_SOCKET].fd = SD_LISTEN_FDS_START;
 pollfd[FD_SOCKET].events = POLLIN;
diff --git a/src/power/sleep.c b/src/power/sleep.c
index cc1ffa6..2462082 100644
--- a/src/power/sleep.c
+++ b/src/power/sleep.c
@@ -200,17 +200,25 @@ int main(int argc, char *argv[]) {
 _cleanup_strv_free_ char **modes = NULL, **states = NULL;
 int r;
 
+/* we are executed through
+ *   - systemd-suspend.service
+ *   - systemd-hibernate.service
+ *   - systemd-hybrid-sleep.service
+ */
+assert(getppid() == 1);
+
 log_set_target(LOG_TARGET_AUTO);
 log_parse_environment();
 log_open();
 
 r = parse_argv(argc, argv);
-if (r <= 0)
-goto finish;
+assert(r > 0);
 
 r = parse_sleep_config(arg_verb, &modes, &states);
-if (r < 0)
+if (r < 0) {
+log_error_errno(r, "Failed to parse " PKGSYSCONFDIR 
"/sleep.conf: %m");
 goto finish;
+}
 
 r = execute(modes, states);
 
-- 
2.2.1.209.g41e5f3a

___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

[systemd-devel] [PATCH 1/8] man: these binaries are internal APIs

[Shawn Landden]

---
 man/systemd-halt.service.xml  | 1 -
 man/systemd-shutdownd.service.xml | 1 -
 man/systemd-suspend.service.xml   | 1 -
 3 files changed, 3 deletions(-)

diff --git a/man/systemd-halt.service.xml b/man/systemd-halt.service.xml
index c94e2a1..7e7f8f2 100644
--- a/man/systemd-halt.service.xml
+++ b/man/systemd-halt.service.xml
@@ -56,7 +56,6 @@
 systemd-poweroff.service
 systemd-reboot.service
 systemd-kexec.service
-/usr/lib/systemd/systemd-shutdown
   
 
   
diff --git a/man/systemd-shutdownd.service.xml 
b/man/systemd-shutdownd.service.xml
index 756949c..051d2ab 100644
--- a/man/systemd-shutdownd.service.xml
+++ b/man/systemd-shutdownd.service.xml
@@ -52,7 +52,6 @@
   
 systemd-shutdownd.service
 systemd-shutdownd.socket
-/usr/lib/systemd/systemd-shutdownd
   
 
   
diff --git a/man/systemd-suspend.service.xml b/man/systemd-suspend.service.xml
index a8beb86..8e3df5f 100644
--- a/man/systemd-suspend.service.xml
+++ b/man/systemd-suspend.service.xml
@@ -56,7 +56,6 @@
 systemd-suspend.service
 systemd-hibernate.service
 systemd-hybrid-sleep.service
-/usr/lib/systemd/system-sleep
   
 
   
-- 
2.2.1.209.g41e5f3a

___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

[systemd-devel] [PATCH 2/8] man/systemd.timer.xml: improve documentation of WakeSystem=

[Shawn Landden]

---
 man/systemd.timer.xml | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/man/systemd.timer.xml b/man/systemd.timer.xml
index 20890f2..4207be0 100644
--- a/man/systemd.timer.xml
+++ b/man/systemd.timer.xml
@@ -230,8 +230,9 @@
 be suspended and if the system supports this. Note that this
 option will only make sure the system resumes on the
 appropriate times, it will not take care of suspending it
-again after any work that is to be done is finished. Defaults
-to false.
+again after any work that is to be done is finished. (For that
+use ExecStartPost=systemd-suspend.service in the 
triggered Unit=)
+Defaults to false.
   
 
   
-- 
2.2.1.209.g41e5f3a

___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

[systemd-devel] [PATCH 4/8] update TODO

[Shawn Landden]

---
 TODO | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/TODO b/TODO
index 52a32d3..bf66ba1 100644
--- a/TODO
+++ b/TODO
@@ -32,6 +32,8 @@ External:
 * When lz4 gets an API for lz4 command output, make use of it to
   compress coredumps in a way compatible with /usr/bin/lz4.
 
+* Fix emacs for Lennart so we can get rid of the Makefile hack littering git
+
 Features:
 
 * journalctl --verify: don't show files that are currently being
-- 
2.2.1.209.g41e5f3a

___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

[systemd-devel] [PATCH 7/8] add rpmvercmp()

[Shawn Landden]

---
 Makefile.am|   2 +
 src/shared/rpmvercmp.c | 122 +
 src/shared/rpmvercmp.h |  14 ++
 3 files changed, 138 insertions(+)
 create mode 100644 src/shared/rpmvercmp.c
 create mode 100644 src/shared/rpmvercmp.h

diff --git a/Makefile.am b/Makefile.am
index e0813a6..1310a20 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -900,6 +900,8 @@ libsystemd_shared_la_SOURCES = \
src/shared/nss-util.h \
src/shared/verbs.c \
src/shared/verbs.h \
+   src/shared/rpmvercmp.c \
+   src/shared/rpmvercmp.h \
src/shared/sigbus.c \
src/shared/sigbus.h \
src/shared/rbtree.c \
diff --git a/src/shared/rpmvercmp.c b/src/shared/rpmvercmp.c
new file mode 100644
index 000..c69c2e3
--- /dev/null
+++ b/src/shared/rpmvercmp.c
@@ -0,0 +1,122 @@
+/* From rpm (Library GPLv2+, which is compatible with LGPLv2.1+)
+ * git://rpm.org/rpm.git
+ */
+
+#include 
+#include 
+#include 
+
+#include "util.h"
+#include "rpmvercmp.h"
+
+/* compare alpha and numeric segments of two versions */
+/* return 1: a is newer than b */
+/*0: a and b are the same version */
+/*   -1: b is newer than a */
+int rpmvercmp(const char * a, const char * b)
+{
+/* easy comparison to see if versions are identical */
+if (streq_ptr(a, b)) return 0;
+
+char oldch1, oldch2;
+char abuf[strlen(a)+1], bbuf[strlen(b)+1];
+char *str1 = abuf, *str2 = bbuf;
+char * one, * two;
+int rc;
+int isnum;
+
+strcpy(str1, a);
+strcpy(str2, b);
+
+one = str1;
+two = str2;
+
+/* loop through each version segment of str1 and str2 and compare them */
+while (*one || *two) {
+   while (*one && !isalnum(*one) && *one != '~') one++;
+   while (*two && !isalnum(*two) && *two != '~') two++;
+
+   /* handle the tilde separator, it sorts before everything else */
+   if (*one == '~' || *two == '~') {
+   if (*one != '~') return 1;
+   if (*two != '~') return -1;
+   one++;
+   two++;
+   continue;
+   }
+
+   /* If we ran to the end of either, we are finished with the loop */
+   if (!(*one && *two)) break;
+
+   str1 = one;
+   str2 = two;
+
+   /* grab first completely alpha or completely numeric segment */
+   /* leave one and two pointing to the start of the alpha or numeric */
+   /* segment and walk str1 and str2 to end of segment */
+   if (isdigit(*str1)) {
+   while (*str1 && isdigit(*str1)) str1++;
+   while (*str2 && isdigit(*str2)) str2++;
+   isnum = 1;
+   } else {
+   while (*str1 && isalpha(*str1)) str1++;
+   while (*str2 && isalpha(*str2)) str2++;
+   isnum = 0;
+   }
+
+   /* save character at the end of the alpha or numeric segment */
+   /* so that they can be restored after the comparison */
+   oldch1 = *str1;
+   *str1 = '\0';
+   oldch2 = *str2;
+   *str2 = '\0';
+
+   /* this cannot happen, as we previously tested to make sure that */
+   /* the first string has a non-null segment */
+   if (one == str1) return -1; /* arbitrary */
+
+   /* take care of the case where the two version segments are */
+   /* different types: one numeric, the other alpha (i.e. empty) */
+   /* numeric segments are always newer than alpha segments */
+   /* XXX See patch #60884 (and details) from bugzilla #50977. */
+   if (two == str2) return (isnum ? 1 : -1);
+
+   if (isnum) {
+   size_t onelen, twolen;
+   /* this used to be done by converting the digit segments */
+   /* to ints using atoi() - it's changed because long  */
+   /* digit segments can overflow an int - this should fix that. */
+
+   /* throw away any leading zeros - it's a number, right? */
+   while (*one == '0') one++;
+   while (*two == '0') two++;
+
+   /* whichever number has more digits wins */
+   onelen = strlen(one);
+   twolen = strlen(two);
+   if (onelen > twolen) return 1;
+   if (twolen > onelen) return -1;
+   }
+
+   /* strcmp will return which one is greater - even if the two */
+   /* segments are alpha or if they are numeric.  don't return  */
+   /* if they are equal because there might be more segments to */
+   /* compare */
+   rc = strcmp(one, two);
+   if (rc) return (rc < 1 ? -1 : 1);
+
+   /* restore character that was replaced by null above */
+   *str1 = oldch1;
+   one = str1;
+   *str2 = oldch2;
+   two = str2;
+}
+
+/* this catches the case where all numeric and alpha segments have */
+/* compared identically but the segment sepparating characters were */
+/* different */
+if ((!*one) && (!*two)) return 0;
+
+/* whichever version still has characters left over wins */
+if (!*one) return -1; else return 1;
+}
diff --git a/src/shared/

[systemd-devel] [PATCH 8/8] shutdown: add kexec loading, avoid calling `kexec` binary unnessecarily

[Shawn Landden]

-82,7 +82,14 @@ void strv_clear(char **l) {
 }
 
 void strv_free(char **l) {
-strv_clear(l);
+char **k;
+
+if (!l)
+return;
+
+for (k = l; *k; k++)
+free(*k);
+
 free(l);
 }
 
diff --git a/src/systemctl/bootspec.c b/src/systemctl/bootspec.c
new file mode 100644
index 000..8673957
--- /dev/null
+++ b/src/systemctl/bootspec.c
@@ -0,0 +1,195 @@
+/*-*- Mode: C; c-basic-offset: 8; indent-tabs-mode: nil -*-*/
+
+/***
+  This file is part of systemd.
+
+  Copyright 2015 Shawn Landden
+
+  systemd is free software; you can redistribute it and/or modify it
+  under the terms of the GNU Lesser General Public License as published by
+  the Free Software Foundation; either version 2.1 of the License, or
+  (at your option) any later version.
+
+  systemd is distributed in the hope that it will be useful, but
+  WITHOUT ANY WARRANTY; without even the implied warranty of
+  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+  Lesser General Public License for more details.
+
+  You should have received a copy of the GNU Lesser General Public License
+  along with systemd; If not, see <http://www.gnu.org/licenses/>.
+***/
+
+/*
+ * Implements http://freedesktop.org/wiki/Specifications/BootLoaderSpec/
+ * for use with kexec
+ */
+
+#include 
+#include 
+#include 
+#include 
+
+#include "bootspec.h"
+#include "strv.h"
+#include "fileio.h"
+#include "rpmvercmp.h"
+
+void bootspec_free(struct BootSpec *s) {
+if (!s)
+return;
+
+free(s->conf);
+free(s);
+}
+
+static int bootspec_cmp(const struct rbtree_node *left, const struct 
rbtree_node *right) {
+struct BootSpec *l = rbtree_container_of( left, struct BootSpec, node),
+*r = rbtree_container_of(right, struct BootSpec, node);
+
+return rpmvercmp(l->version, r->version);
+}
+
+int kernel_bootloaderspec_readconf(struct rbtree *tree) {
+int r = 0;
+_cleanup_closedir_ DIR *entries;
+struct dirent *dir;
+
+rbtree_init(tree, bootspec_cmp, 0);
+
+entries = opendir(BOOTENTRIESDIR);
+if (!entries) {
+if (r == -ENOENT)
+return 0;
+else
+return -errno;
+}
+
+for (size_t i=0;(dir = readdir(entries));i++) {
+struct BootSpec *entry;
+/*   compiler wont allow 256 + strlen(BOOTENTRIESDIR) here */
+char fn[512] = {BOOTENTRIESDIR}, *d_name = 
&fn[strlen(BOOTENTRIESDIR)],
+ *m, *l, *k;
+
+if (!endswith(dir->d_name, ".conf")) {
+i--;
+continue;
+}
+
+entry = new0(struct BootSpec, 1);
+if (!entry)
+return -ENOMEM;
+
+(void)strcpy(d_name, dir->d_name);
+
+r = read_full_file(fn, &entry->conf, NULL);
+if (r < 0)
+return -errno;
+
+for (m = entry->conf; ; m = k + 1) {
+if (m[0] == '#')
+continue;
+
+k = strchr(m, '\n');
+
+if (k)
+*k = '\0';
+else
+break;
+
+if  ((l = startswith(m, "title ")))
+entry->title  = l + strspn(l, WHITESPACE);
+else if ((l = startswith(m, "version ")))
+entry->version= l + strspn(l, WHITESPACE);
+else if ((l = startswith(m, "machine-id ")))
+(void)sd_id128_from_string(l + strspn(l, 
WHITESPACE), &entry->machine_id);
+else if ((l = startswith(m, "options ")))
+entry->options= l + strspn(l, WHITESPACE);
+else if ((l = startswith(m, "linux ")))
+entry->linux_loc  = l + strspn(l, WHITESPACE);
+else if ((l = startswith(m, "initrd ")))
+entry->initrd = l + strspn(l, WHITESPACE);
+else if ((l = startswith(m, "efi ")))
+entry->efi= l + strspn(l, WHITESPACE);
+else if ((l = startswith(m, "devicetree ")))
+entry->devicetree = l + strspn(l, WHITESPACE);
+else
+continue;
+}
+
+/* not interested in EFI programs */
+if (!entry->

[systemd-devel] [v1] shutdown: add kexec loading, avoid calling `kexec` binary unnessecarily

[Shawn Landden]

r Dom0 and DomU but we only get Dom0
+ * because of the !in_container check */
+if (access("/proc/xen", F_OK) == 0) {
+pid_t pid;
 
 pid = fork();
 if (pid < 0)
@@ -370,7 +376,9 @@ int main(int argc, char *argv[]) {
 _exit(EXIT_FAILURE);
 } else
 wait_for_terminate_and_warn("kexec", pid, 
true);
-}
+
+} else
+reboot(cmd);
 
 cmd = RB_AUTOBOOT;
 /* Fall through */
diff --git a/src/shared/missing.h b/src/shared/missing.h
index b33a70c..d80b2a7 100644
--- a/src/shared/missing.h
+++ b/src/shared/missing.h
@@ -35,6 +35,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #ifdef HAVE_AUDIT
 #include 
@@ -763,3 +764,13 @@ static inline int kcmp(pid_t pid1, pid_t pid2, int type, 
unsigned long idx1, uns
 #ifndef KCMP_FILE
 #define KCMP_FILE 0
 #endif
+
+/* v3.17 */
+#ifndef __NR_kexec_file_load
+#ifdef __x86_64__
+#define __NR_kexec_file_load 320
+#endif
+#endif
+#ifndef KEXEC_FILE_NO_INITRAMFS
+#define KEXEC_FILE_NO_INITRAMFS0x0004
+#endif
diff --git a/src/shared/rpmvercmp.c b/src/shared/rpmvercmp.c
index c69c2e3..09cfd97 100644
--- a/src/shared/rpmvercmp.c
+++ b/src/shared/rpmvercmp.c
@@ -13,8 +13,16 @@
 /* return 1: a is newer than b */
 /*0: a and b are the same version */
 /*   -1: b is newer than a */
-int rpmvercmp(const char * a, const char * b)
-{
+int rpmvercmp(const char * a, const char * b) {
+if (!a) {
+if (b)
+return -1;
+else
+return 0;
+}
+if (!b)
+return 1;
+
 /* easy comparison to see if versions are identical */
 if (streq_ptr(a, b)) return 0;
 
diff --git a/src/shared/strv.c b/src/shared/strv.c
index e27ac68..d983665 100644
--- a/src/shared/strv.c
+++ b/src/shared/strv.c
@@ -82,7 +82,14 @@ void strv_clear(char **l) {
 }
 
 void strv_free(char **l) {
-strv_clear(l);
+char **k;
+
+if (!l)
+return;
+
+for (k = l; *k; k++)
+free(*k);
+
 free(l);
 }
 
diff --git a/src/systemctl/bootspec.c b/src/systemctl/bootspec.c
new file mode 100644
index 000..0a43c7b
--- /dev/null
+++ b/src/systemctl/bootspec.c
@@ -0,0 +1,204 @@
+/*-*- Mode: C; c-basic-offset: 8; indent-tabs-mode: nil -*-*/
+
+/***
+  This file is part of systemd.
+
+  Copyright 2015 Shawn Landden
+
+  systemd is free software; you can redistribute it and/or modify it
+  under the terms of the GNU Lesser General Public License as published by
+  the Free Software Foundation; either version 2.1 of the License, or
+  (at your option) any later version.
+
+  systemd is distributed in the hope that it will be useful, but
+  WITHOUT ANY WARRANTY; without even the implied warranty of
+  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+  Lesser General Public License for more details.
+
+  You should have received a copy of the GNU Lesser General Public License
+  along with systemd; If not, see <http://www.gnu.org/licenses/>.
+***/
+
+/*
+ * Implements http://freedesktop.org/wiki/Specifications/BootLoaderSpec/
+ * for use with kexec
+ */
+
+#include 
+#include 
+#include 
+#include 
+
+#include "bootspec.h"
+#include "strv.h"
+#include "fileio.h"
+#include "rpmvercmp.h"
+
+void bootspec_free(struct BootSpec *s) {
+if (!s)
+return;
+
+free(s->conf);
+free(s);
+}
+
+static int bootspec_cmp(const struct rbtree_node *left, const struct 
rbtree_node *right) {
+struct BootSpec *l = rbtree_container_of( left, struct BootSpec, node),
+*r = rbtree_container_of(right, struct BootSpec, node);
+
+return rpmvercmp(l->version, r->version);
+}
+
+int kernel_bootloaderspec_readconf(struct rbtree *tree) {
+int r = 0;
+_cleanup_closedir_ DIR *entries;
+struct dirent *dir;
+
+rbtree_init(tree, bootspec_cmp, 0);
+
+entries = opendir(BOOTENTRIESDIR);
+if (!entries) {
+if (r == -ENOENT)
+return 0;
+else
+return -errno;
+}
+
+for (size_t i=0;(dir = readdir(entries));i++) {
+struct BootSpec *entry;
+/*   compiler wont allow 256 + strlen(BOOTENTRIESDIR) here */
+char fn[512] = {BOOTENTRIESDIR}, *d_name = 
&fn[strlen(BOOTENTRIESDIR)],
+ *m, *l, *k;
+
+if (!endswith(dir->d_name, ".conf")) {
+i--;
+continue;
+}
+
+entry = new0(struct BootSpec, 1);
+if (!entry)
+  

Re: [systemd-devel] A use case for staged startup

[Shawn Landden]

On Sat, Feb 21, 2015 at 7:31 AM, Jeff Waugh  wrote:

> Hi all,
>
> Marko Hoyer recently brought up the concept of a staged startup [1] on
> this list.
>
> I have a specific use case for some form of staging, though I don't know
> if it meets Marko's definition or requirements! Perhaps systemd can handle
> this already, but let's see...
>
>
> So, I've been building a systemd package for OpenWrt [2] to test on my
> little VoCore coin-sized MIPS machine. (Stay with me, the weird part is
> over.)
>
> The root filesystem is a read-only squashfs blob stored on the VoCore's
> generous (!) 8MB of flash memory. During initial testing, I was happy to
> boot up into a read-only environment, bring up a few tmpfs mount points,
> and then keep mucking around with systemd.
>
> But it's time to get serious. And everyone knows that "serious" means
> having a writeable root filesystem. OpenWrt uses overlayfs with JFFS2 as
> the top layer. but I'm just using tmpfs for now. (For some values of
> "serious".)
>
>
> I wanted to make best use of systemd's built-in primitives, so here's what
> I've done:
>
> - default.target is symlinked to initrd.target in the read-only filesystem
> image
>
> - I've added some custom services to prepare all the mounts for the root
> switch (including one which changes the default.target symlink on the new,
> writeable root)
>
> Yes, I'm abusing systemd's idea of an initrd.
>
>
> Here's where it breaks down:
>
> - systemd dutifully starts all the services it knows about during the
> initrd.target run, because they're all right there on the read-only
> filesystem (and they fail a lot)
>
> - then systemd dutifully stops them all again to switch the root
>
> - and dutifully starts them all again once we're headed towards
> multi-user.target
>
> That's a *lot* of noise in the startup process!
>
>
> I did get the impression from the documentation that initrd.target was
> somehow special, but it makes complete sense that it's not. If I were using
> an initramfs, there wouldn't be any superfluous service files in the
> initramfs filesystem, and I'd be happy to know that systemd would behave
> *exactly* the same way it would elsewhere.
>
>
> One hacky idea I had to fix this:
>
> - Pull all of the systemd service symlinks out of the squashfs filesystem
> and store them in a tarball
>
> - Add specific symlinks to make the initrd stage works properly
>
> - In the pre-switch prepare service, unpack the tarball into
> the rw-mounted /sysroot
>
>
> Before anyone says it: No, using a real initramfs would be highly
> inappropriate. I do not want to store two copies of systemd and friends in
> 8MB of flash. It's hard enough with just the one!
>
>
> Is there an existing systemd solution to this problem? Is there a better
> way to go about it?
>
> You could try a bazillion bind mounts from the initrd to the target
filesystem, and then calling switch_root(8) from util-linux, and you do not
need systemctl like the previous post suggests, if you get the dependencies
right. You might also look at overlayfs, which got merged in Linux 3.18. I
think overlayfs will make this possible, and of course share your results!

> Thanks,
> Jeff
>
> [1]
> http://lists.freedesktop.org/archives/systemd-devel/2015-January/027688.html
> [2] https://github.com/jdub/openwrt-systemd
>
> ___
> systemd-devel mailing list
> systemd-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/systemd-devel
>
>


-- 

---
Shawn Landden
ChurchOfGit.com
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] feature request: dlopen

[Shawn Landden]

 to significant success - the situation surrounding systemd as
> a sales pitch to have GNU/linux systems successfully replaced with
> windows servers.  that GNU/linux is being relegated to the hypervisor
> / management role of windows systems which are successfully claimed to
> be much more dependable.  systemd is being used to successfully
> *scare* people into buying windows - back into the arms of the
> monopoly power i worked incredibly hard to give people a way to move
> away from.  i have to say: I'm Officially Not Happy With You (tm) for
> that :)
>
> i hope... my hope is that, by providing you with these insights and a
> potential and easily-deployed solution (in going through all the
> packages and hard-removing libsystemd0, adam was in a unique position
> to evaluate the dlopen option and found it to be technically easy to
> do), i hope that i have shocked you into taking immediate action.  my
> hope here is that you will realise the gravity and enormity of the
> situation that the software libre world faces right now, sufficient
> that you will give this absolute top priority above everything else
> that cannot be immediately put on hold.
>
> i am subscribed to this list on "nomail", and will follow it on gmane.
> as you are experienced software developers i would not presume to
> interfere with how to go about dynamically-loading of libsystemd0,
> however if you would appreciate the benefit of my experience (which
> comes in part from one of the software libre world's more experienced
> unix portability experts, andrew tridgell), i will be more than happy
> to help as i can.
>
> l.
> ___
> systemd-devel mailing list
> systemd-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/systemd-devel
>



-- 

---
Shawn Landden
ChurchOfGit.com
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [PATCH] configure: turn off -Wlogical-not-parentheses

[Shawn Landden]

On Tue, Feb 24, 2015 at 9:33 AM, David Herrmann 
wrote:

> Hi
>
> On Mon, Feb 16, 2015 at 11:02 PM, Shawn Landden 
> wrote:
> > Introduced in gcc-5
> >
> > These errors are really annoying. I can get behind clarification of
> nested ifs,
> > but this is overkill.
> > ---
> >  configure.ac | 1 +
> >  1 file changed, 1 insertion(+)
> >
> > diff --git a/configure.ac b/configure.ac
> > index 97a29d6..e646db7 100644
> > --- a/configure.ac
> > +++ b/configure.ac
> > @@ -187,6 +187,7 @@ CC_CHECK_FLAGS_APPEND([with_cflags], [CFLAGS], [\
> >  -Wno-unused-parameter \
> >  -Wno-missing-field-initializers \
> >  -Wno-unused-result \
> > +-Wno-logical-not-parentheses \
> >  -Werror=overflow \
> >  -Wdate-time \
> >  -Wnested-externs \
>
> I fixed the -Wno-* detection today. Is this patch still needed? I
> don't have gcc5 here, but I think Daniel fixed most of those warnings
> recently and they turned out to be real bugs. So should I still apply
> it?
>
> Not if it detected real bugs. The warnings I saw did not appear to be
bugsbut perhaps that was the ones in the linux tree and not systemd...

> Thanks
> David
>
> > --
> > 2.2.1.209.g41e5f3a
> >
> > ___
> > systemd-devel mailing list
> > systemd-devel@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/systemd-devel
> ___
> systemd-devel mailing list
> systemd-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/systemd-devel
>



-- 
Shawn Landden
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [PATCH 3/8] power: refactor the three power management binaries to src/power

[Shawn Landden]

On Thu, Feb 26, 2015 at 6:26 PM, Zbigniew Jędrzejewski-Szmek <
zbys...@in.waw.pl> wrote:

> I'm not sure we want this... Can you add some justification? Do they share
> code?
>
I found it confusing what parts each of these handled, and some code
sharing is possible, but none is shared currently. This also eliminates one
directory. All of these are handled by units:

sleep.target
shutdown.target
etc...

>
> On Fri, Feb 20, 2015 at 02:31:00PM -0800, Shawn Landden wrote:
> > ---
> >  Makefile.am   |   6 +-
> >  src/core/shutdown.c   | 420
> -
> >  src/power/Makefile|  28 +++
> This should be a symlink.
>
> To make all of these symlinks would be a much larger patch, but I can send
such a patch

> >  src/power/shutdown.c  | 420
> +
> >  src/power/shutdownd.c | 461
> ++
> >  src/power/sleep.c | 219 ++
> >  src/shutdownd/Makefile|   1 -
> >  src/shutdownd/shutdownd.c | 461
> --
> >  src/sleep/Makefile|   1 -
> >  src/sleep/sleep.c | 219 --
> >  10 files changed, 1131 insertions(+), 1105 deletions(-)
> >  delete mode 100644 src/core/shutdown.c
> >  create mode 100644 src/power/Makefile
> >  create mode 100644 src/power/shutdown.c
> >  create mode 100644 src/power/shutdownd.c
> >  create mode 100644 src/power/sleep.c
> >  delete mode 12 src/shutdownd/Makefile
> >  delete mode 100644 src/shutdownd/shutdownd.c
> >  delete mode 12 src/sleep/Makefile
> >  delete mode 100644 src/sleep/sleep.c
> It's better to use -M for such patches... Make it easier to see what is
> hapenning.
>
> Zbyszek
> ___
> systemd-devel mailing list
> systemd-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/systemd-devel
>



-- 
Shawn Landden
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [v1] shutdown: add kexec loading, ?avoid calling `kexec` binary unnessecarily

[Shawn Landden]

On Thu, Feb 26, 2015 at 6:22 PM, Zbigniew Jędrzejewski-Szmek <
zbys...@in.waw.pl> wrote:

> On Thu, Feb 26, 2015 at 08:04:08AM +, Jan Janssen wrote:
> > Shawn Landden  churchofgit.com> writes:
> >
> > >  void strv_free(char **l) {
> > > -strv_clear(l);
> > > +char **k;
> > > +
> > > +if (!l)
> > > +return;
> > > +
> > > +for (k = l; *k; k++)
> > > +free(*k);
> > > +
> > >  free(l);
> > >  }
> > What are you trying to achieve here? I see no point in optimizing out
> the *l
> > = NULL from strv_clear.
> >
> > > +entry->linux_loc  = l + strspn(l,
> > WHITESPACE);
> > > +else if ((l = startswith(m, "initrd ")))
> > > +entry->initrd = l + strspn(l,
> > WHITESPACE);
> > You need to support more than one initrd per kernel, see
> > https://wiki.archlinux.org/index.php/Microcode for why. Also, I am
> pretty
> > sure you can have a initrd=/path/to/initrd in the kernel options entry.
> > Since the efi bootloader just appends each given initrd to the kernel
> > command line.
> >
> >
> > All in all I am wondering why you need a rbtree for all this in the first
> > place? A simple hashmap should do just fine.
> A hashmap does not keep order. But a simple array + qsort_safe() should
> work too. I'm wary of introducing yet another data structure into systemd
> which raises the bar for people editing the code later on or making
> changes.
>
> We need two operations: sorting kernels to list them, and picking (I
> presume) the
> latest kernel or the kernel with the given version. Both of those
> operations
> are done once over the lifetime of the program, so any speedup in using
> a data structure should take into account the time to set up the structure.
> Neither of those operations is speed sensitive, and the more common
> operation
> of picking a specific version can be done in O(n) over an array. So using
> an rbtree will not save any time actually.
>
I was initially using a vector of pointers here for the same reasons you
reiterated, but I felt the use of greedy_realloc0() was messy and
error-prone. The rbtree does not require the use of realloc(). There is no
way to know how long the array needs to be from the start. Even the O(n)
you mention could be turned into O(logn) by using a binary search.

>
> > Also, you're not taking multi-boot into account (the machine-id field).
> > You're just discriminating based on the kernel version, but different
> > installations could have the same version field.
> This high-level design questions should probably be addressed first...
>
> Zbyszek
> ___
> systemd-devel mailing list
> systemd-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/systemd-devel
>



-- 
Shawn Landden
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [v1] shutdown: add kexec loading, ?avoid calling `kexec` binary unnessecarily

[Shawn Landden]

On Fri, Feb 27, 2015 at 9:03 AM, Zbigniew Jędrzejewski-Szmek <
zbys...@in.waw.pl> wrote:
> > We need two operations: sorting kernels to list them, and picking (I

> On Fri, Feb 27, 2015 at 08:58:04AM -0800, Shawn Landden wrote:
> > On Thu, Feb 26, 2015 at 6:22 PM, Zbigniew Jędrzejewski-Szmek <
> > zbys...@in.waw.pl> wrote:
> >
> > > On Thu, Feb 26, 2015 at 08:04:08AM +, Jan Janssen wrote:
> > > > Shawn Landden  churchofgit.com> writes:
> > > >
> > > > >  void strv_free(char **l) {
> > > > > -strv_clear(l);
> > > > > +char **k;
> > > > > +
> > > > > +if (!l)
> > > > > +return;
> > > > > +
> > > > > +for (k = l; *k; k++)
> > > > > +free(*k);
> > > > > +
> > > > >  free(l);
> > > > >  }
> > > > What are you trying to achieve here? I see no point in optimizing out
> > > the *l
> > > > = NULL from strv_clear.
> > > >
> > > > > +entry->linux_loc  = l + strspn(l,
> > > > WHITESPACE);
> > > > > +else if ((l = startswith(m, "initrd ")))
> > > > > +entry->initrd = l + strspn(l,
> > > > WHITESPACE);
> > > > You need to support more than one initrd per kernel, see
> > > > https://wiki.archlinux.org/index.php/Microcode for why. Also, I am
> > > pretty
> > > > sure you can have a initrd=/path/to/initrd in the kernel options
> entry.
> > > > Since the efi bootloader just appends each given initrd to the kernel
> > > > command line.
> > > >
> > > >
> > > > All in all I am wondering why you need a rbtree for all this in the
> first
> > > > place? A simple hashmap should do just fine.
> > > A hashmap does not keep order. But a simple array + qsort_safe() should
> > > work too. I'm wary of introducing yet another data structure into
> systemd
> > > which raises the bar for people editing the code later on or making
> > > changes.
> > >
> > > presume) the
> > > latest kernel or the kernel with the given version. Both of those
> > > operations
> > > are done once over the lifetime of the program, so any speedup in using
> > > a data structure should take into account the time to set up the
> structure.
> > > Neither of those operations is speed sensitive, and the more common
> > > operation
> > > of picking a specific version can be done in O(n) over an array. So
> using
> > > an rbtree will not save any time actually.
> > >
> > I was initially using a vector of pointers here for the same reasons you
> > reiterated, but I felt the use of greedy_realloc0() was messy and
> > error-prone.
> greedy_realloc0() is not that messy. And it would be just a few lines
> of code. We have similar patterns in many other places, and
> consistency is good.
>
> > The rbtree does not require the use of realloc(). There is no
> > way to know how long the array needs to be from the start. Even the O(n)
> > you mention could be turned into O(logn) by using a binary search.
> Nope. The array needs to be sorted to do a binary search. So the
> upfront cost of sorting kills any gain you get later on.
>
Oh I see, you don't have to sort in that case. But the code would be
longer, so perhaps I will sort in all cases.

>
> Zbyszek
> ___
> systemd-devel mailing list
> systemd-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/systemd-devel
>



-- 
Shawn Landden
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

[systemd-devel] sd_id128_equal borked

[Shawn Landden]

_sd_pure_ static inline int sd_id128_equal(sd_id128_t a, sd_id128_t b) {
return memcmp(&a, &b, 16) == 0;
}

this should either be return memcmp(&a, &b, 16);
or return bool

-- 

---
Shawn Landden
ChurchOfGit.com
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [v1] shutdown: add kexec loading, avoid calling `kexec` binary unnessecarily

[Shawn Landden]

On Thu, Feb 26, 2015 at 12:04 AM, Jan Janssen  wrote:

> Shawn Landden  churchofgit.com> writes:
>
> >  void strv_free(char **l) {
> > -strv_clear(l);
> > +char **k;
> > +
> > +if (!l)
> > +return;
> > +
> > +for (k = l; *k; k++)
> > +free(*k);
> > +
> >  free(l);
> >  }
> What are you trying to achieve here? I see no point in optimizing out the
> *l
> = NULL from strv_clear.
>
> > +entry->linux_loc  = l + strspn(l,
> WHITESPACE);
> > +else if ((l = startswith(m, "initrd ")))
> > +entry->initrd = l + strspn(l,
> WHITESPACE);
> You need to support more than one initrd per kernel, see
> https://wiki.archlinux.org/index.php/Microcode for why. Also, I am pretty
> sure you can have a initrd=/path/to/initrd in the kernel options entry.
> Since the efi bootloader just appends each given initrd to the kernel
> command line.
>
I can't support more than one initrd per kernel with the kexec_file_load()
syscall, and if initrd on the commandline works, then it will still work
with this patch, so i don't need to change anything.

>
>
> All in all I am wondering why you need a rbtree for all this in the first
> place? A simple hashmap should do just fine.
>
> Also, you're not taking multi-boot into account (the machine-id field).
> You're just discriminating based on the kernel version, but different
> installations could have the same version field.
>
fixed by testing that the machine-id is the same (I forgot this part of the
spec thanks). Is there anyway I should save defaults? Is there anything in
the spec that is missing? Perhaps it should specify how to save last-boot.

>
> Jan
>
> ___
> systemd-devel mailing list
> systemd-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/systemd-devel
>



-- 

---
Shawn Landden
ChurchOfGit.com
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

[systemd-devel] [v3 1/4] man: these binaries are internal APIs

[Shawn Landden]

---
 man/systemd-halt.service.xml  | 1 -
 man/systemd-shutdownd.service.xml | 1 -
 man/systemd-suspend.service.xml   | 1 -
 3 files changed, 3 deletions(-)

diff --git a/man/systemd-halt.service.xml b/man/systemd-halt.service.xml
index c94e2a1..7e7f8f2 100644
--- a/man/systemd-halt.service.xml
+++ b/man/systemd-halt.service.xml
@@ -56,7 +56,6 @@
 systemd-poweroff.service
 systemd-reboot.service
 systemd-kexec.service
-/usr/lib/systemd/systemd-shutdown
   
 
   
diff --git a/man/systemd-shutdownd.service.xml 
b/man/systemd-shutdownd.service.xml
index 756949c..051d2ab 100644
--- a/man/systemd-shutdownd.service.xml
+++ b/man/systemd-shutdownd.service.xml
@@ -52,7 +52,6 @@
   
 systemd-shutdownd.service
 systemd-shutdownd.socket
-/usr/lib/systemd/systemd-shutdownd
   
 
   
diff --git a/man/systemd-suspend.service.xml b/man/systemd-suspend.service.xml
index a8beb86..8e3df5f 100644
--- a/man/systemd-suspend.service.xml
+++ b/man/systemd-suspend.service.xml
@@ -56,7 +56,6 @@
 systemd-suspend.service
 systemd-hibernate.service
 systemd-hybrid-sleep.service
-/usr/lib/systemd/system-sleep
   
 
   
-- 
2.2.1.209.g41e5f3a

___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

[systemd-devel] [v3 2/4] update TODO

[Shawn Landden]

---
 TODO | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/TODO b/TODO
index e8e4800..1f5bfaa 100644
--- a/TODO
+++ b/TODO
@@ -32,6 +32,8 @@ External:
 * When lz4 gets an API for lz4 command output, make use of it to
   compress coredumps in a way compatible with /usr/bin/lz4.
 
+* Fix emacs for Lennart so we can get rid of the Makefile hack littering git
+
 Features:
 
 * When logging about multiple units (stopping BoundTo units, conflicts, etc.),
-- 
2.2.1.209.g41e5f3a

___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

[systemd-devel] [v3 3/4] add rpmvercmp()

[Shawn Landden]

---
 Makefile.am|   2 +
 src/shared/rpmvercmp.c | 122 +
 src/shared/rpmvercmp.h |  14 ++
 3 files changed, 138 insertions(+)
 create mode 100644 src/shared/rpmvercmp.c
 create mode 100644 src/shared/rpmvercmp.h

diff --git a/Makefile.am b/Makefile.am
index e77a242..bba5353 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -902,6 +902,8 @@ libsystemd_shared_la_SOURCES = \
src/shared/nss-util.h \
src/shared/verbs.c \
src/shared/verbs.h \
+   src/shared/rpmvercmp.c \
+   src/shared/rpmvercmp.h \
src/shared/sigbus.c \
src/shared/sigbus.h \
src/shared/build.h \
diff --git a/src/shared/rpmvercmp.c b/src/shared/rpmvercmp.c
new file mode 100644
index 000..c69c2e3
--- /dev/null
+++ b/src/shared/rpmvercmp.c
@@ -0,0 +1,122 @@
+/* From rpm (Library GPLv2+, which is compatible with LGPLv2.1+)
+ * git://rpm.org/rpm.git
+ */
+
+#include 
+#include 
+#include 
+
+#include "util.h"
+#include "rpmvercmp.h"
+
+/* compare alpha and numeric segments of two versions */
+/* return 1: a is newer than b */
+/*0: a and b are the same version */
+/*   -1: b is newer than a */
+int rpmvercmp(const char * a, const char * b)
+{
+/* easy comparison to see if versions are identical */
+if (streq_ptr(a, b)) return 0;
+
+char oldch1, oldch2;
+char abuf[strlen(a)+1], bbuf[strlen(b)+1];
+char *str1 = abuf, *str2 = bbuf;
+char * one, * two;
+int rc;
+int isnum;
+
+strcpy(str1, a);
+strcpy(str2, b);
+
+one = str1;
+two = str2;
+
+/* loop through each version segment of str1 and str2 and compare them */
+while (*one || *two) {
+   while (*one && !isalnum(*one) && *one != '~') one++;
+   while (*two && !isalnum(*two) && *two != '~') two++;
+
+   /* handle the tilde separator, it sorts before everything else */
+   if (*one == '~' || *two == '~') {
+   if (*one != '~') return 1;
+   if (*two != '~') return -1;
+   one++;
+   two++;
+   continue;
+   }
+
+   /* If we ran to the end of either, we are finished with the loop */
+   if (!(*one && *two)) break;
+
+   str1 = one;
+   str2 = two;
+
+   /* grab first completely alpha or completely numeric segment */
+   /* leave one and two pointing to the start of the alpha or numeric */
+   /* segment and walk str1 and str2 to end of segment */
+   if (isdigit(*str1)) {
+   while (*str1 && isdigit(*str1)) str1++;
+   while (*str2 && isdigit(*str2)) str2++;
+   isnum = 1;
+   } else {
+   while (*str1 && isalpha(*str1)) str1++;
+   while (*str2 && isalpha(*str2)) str2++;
+   isnum = 0;
+   }
+
+   /* save character at the end of the alpha or numeric segment */
+   /* so that they can be restored after the comparison */
+   oldch1 = *str1;
+   *str1 = '\0';
+   oldch2 = *str2;
+   *str2 = '\0';
+
+   /* this cannot happen, as we previously tested to make sure that */
+   /* the first string has a non-null segment */
+   if (one == str1) return -1; /* arbitrary */
+
+   /* take care of the case where the two version segments are */
+   /* different types: one numeric, the other alpha (i.e. empty) */
+   /* numeric segments are always newer than alpha segments */
+   /* XXX See patch #60884 (and details) from bugzilla #50977. */
+   if (two == str2) return (isnum ? 1 : -1);
+
+   if (isnum) {
+   size_t onelen, twolen;
+   /* this used to be done by converting the digit segments */
+   /* to ints using atoi() - it's changed because long  */
+   /* digit segments can overflow an int - this should fix that. */
+
+   /* throw away any leading zeros - it's a number, right? */
+   while (*one == '0') one++;
+   while (*two == '0') two++;
+
+   /* whichever number has more digits wins */
+   onelen = strlen(one);
+   twolen = strlen(two);
+   if (onelen > twolen) return 1;
+   if (twolen > onelen) return -1;
+   }
+
+   /* strcmp will return which one is greater - even if the two */
+   /* segments are alpha or if they are numeric.  don't return  */
+   /* if they are equal because there might be more segments to */
+   /* compare */
+   rc = strcmp(one, two);
+   if (rc) return (rc < 1 ? -1 : 1);
+
+   /* restore character that was replaced by null above */
+   *str1 = oldch1;
+   one = str1;
+   *str2 = oldch2;
+   two = str2;
+}
+
+/* this catches the case where all numeric and alpha segments have */
+/* compared identically but the segment sepparating characters were */
+/* different */
+if ((!*one) && (!*two)) return 0;
+
+/* whichever version still has characters left over wins */
+if (!*one) return -1; else return 1;
+}
diff --git a/src/shared/r

[systemd-devel] [v3 4/4] shutdown: add kexec loading, avoid calling `kexec` binary unnessecarily

[Shawn Landden]

) {
 }
 
 void strv_free(char **l) {
-strv_clear(l);
+char **k;
+
+if (!l)
+return;
+
+for (k = l; *k; k++)
+free(*k);
+
 free(l);
 }
 
diff --git a/src/systemctl/bootspec.c b/src/systemctl/bootspec.c
new file mode 100644
index 000..f8d1bc5
--- /dev/null
+++ b/src/systemctl/bootspec.c
@@ -0,0 +1,230 @@
+/*-*- Mode: C; c-basic-offset: 8; indent-tabs-mode: nil -*-*/
+
+/***
+  This file is part of systemd.
+
+  Copyright 2015 Shawn Landden
+
+  systemd is free software; you can redistribute it and/or modify it
+  under the terms of the GNU Lesser General Public License as published by
+  the Free Software Foundation; either version 2.1 of the License, or
+  (at your option) any later version.
+
+  systemd is distributed in the hope that it will be useful, but
+  WITHOUT ANY WARRANTY; without even the implied warranty of
+  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+  Lesser General Public License for more details.
+
+  You should have received a copy of the GNU Lesser General Public License
+  along with systemd; If not, see <http://www.gnu.org/licenses/>.
+***/
+
+/*
+ * Implements http://freedesktop.org/wiki/Specifications/BootLoaderSpec/
+ * for use with kexec
+ */
+
+#include 
+#include 
+#include 
+#include 
+
+#include "bootspec.h"
+#include "strv.h"
+#include "fileio.h"
+#include "rpmvercmp.h"
+
+void bootspec_free(struct BootSpec *s) {
+if (!s)
+return;
+
+free(s->conf);
+free(s);
+}
+
+static int bootspec_cmp(const void *left, const void *right) {
+const struct BootSpec *l = left,
+  *r = right;
+
+return rpmvercmp(l->version, r->version);
+}
+
+int bootspec_getkernels(struct BootSpec **ret, size_t *cl) {
+int r = 0;
+struct BootSpec *c;
+_cleanup_closedir_ DIR *d;
+struct dirent *dent;
+size_t i = 1, ii = 2;
+_cleanup_close_ int midfd = -1;
+ssize_t ss;
+char mid_char[32 + 1];
+sd_id128_t machine_id;
+
+midfd = open("/etc/machine-id", O_RDONLY);
+if (midfd < 0)
+return -errno;
+ss = read(midfd, &mid_char, sizeof(mid_char));
+if (ss < 0)
+return -errno;
+else if (ss != 33)
+return -EBADF;
+if (mid_char[32] == '\n')
+mid_char[32] = '\0';
+else
+return -EBADF;
+r = sd_id128_from_string(mid_char, &machine_id);
+if (r < 0)
+return r;
+
+c = new0(struct BootSpec, i + 1);
+if (!c)
+return -ENOMEM;
+
+d = opendir(BOOTENTRIESDIR);
+if (!d) {
+if (errno == ENOENT)
+return 0;
+else
+return -errno;
+}
+
+for (dent = readdir(d); dent != NULL; dent = readdir(d)) {
+struct BootSpec *bs;
+char *m, *l, *k;
+_cleanup_fclose_ FILE *f = NULL;
+
+if (!endswith(dent->d_name, ".conf"))
+continue;
+
+c = greedy_realloc0((void **)&c, &ii, i + 2, sizeof(struct 
BootSpec));
+if (!c)
+return -ENOMEM;
+bs = &c[i - 1];
+
+f = fopenat(dirfd(d), dent->d_name, "r");
+if (!f)
+return -errno;
+
+r = read_full_stream(f, &bs->conf, NULL);
+if (r < 0)
+return r;
+
+for (m = bs->conf; ; m = k + 1) {
+if (m[0] == '#')
+continue;
+
+k = strchr(m, '\n');
+
+if (k)
+*k = '\0';
+else
+break;
+
+if  ((l = startswith(m, "title ")))
+bs->title  = l + strspn(l, WHITESPACE);
+else if ((l = startswith(m, "version ")))
+bs->version= l + strspn(l, WHITESPACE);
+else if ((l = startswith(m, "machine-id ")))
+(void)sd_id128_from_string(l + strspn(l, 
WHITESPACE), &bs->machine_id);
+else if ((l = startswith(m, "options ")))
+bs->options= l + strspn(l, WHITESPACE);
+else if ((l = startswith(m, "linux ")))
+bs->linux_loc  = l + strspn(l, WHITESPACE);
+  

Re: [systemd-devel] Plans to fix or provide alternative for lz4?

[Shawn Landden]

On Sun, Mar 1, 2015 at 3:04 AM, Lennart Poettering 
wrote:

> On Thu, 26.02.15 05:55, Zbigniew Jędrzejewski-Szmek (zbys...@in.waw.pl)
> wrote:
>
> > On Thu, Feb 26, 2015 at 04:41:48AM +, Laszlo Papp wrote:
> > > Hi,
> > >
> > > it seems that the lz4 headers are broken when getting coredumps
> > > generated. They cannot even be extracted by the lz4 tool itself, let
> > > alone using them via the coredump controller util.
> > >
> > > My system, which is Archlinux, is using lz4 127 and systemd 219.
> > >
> > > My current workaround was to disable compression altogether for
> > > coredumps in the corresponding config file, but it is suboptimal,
> > > especially on embedded systems.
> > The headers are different because when lz4 support was added, lz4 did
> > not provide a library to write the headers so I added custom headers.
> > You should be able to use coredumpctl to unpack the file.
> >
> > "Proper" lz4 support has been written, but lz4 upstream has trouble
> > with keeping .so compatibility:
> https://code.google.com/p/lz4/issues/detail?id=147.
> > So the question is whether to replace lz4 with something more stable
> > or to ignore the issue and hope it doesn't happen again.
>
> Maybe gzip might be a good compromise? Due to the importd logic we now
> check for libz anyway in configure.ac, we might as well use it for the
> journal. To my knowledge gzip is still a good compromise between
> compression ratio on one hand and memory use/time on the other...
>
except xz beats gzip at its lower compression settings even on memory
use/time. See man 1 xz.

>
> Lennart
>
> --
> Lennart Poettering, Red Hat
> ___
> systemd-devel mailing list
> systemd-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/systemd-devel
>



-- 

---
Shawn Landden
ChurchOfGit.com
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

[systemd-devel] [PATCH] fix strict aliasing violations in src/udev/udev-builtin-usb_id.c

[Shawn Landden]

also switch to 
---
 src/udev/udev-builtin-usb_id.c | 35 ++-
 1 file changed, 18 insertions(+), 17 deletions(-)

diff --git a/src/udev/udev-builtin-usb_id.c b/src/udev/udev-builtin-usb_id.c
index ab0d96e..0223421 100644
--- a/src/udev/udev-builtin-usb_id.c
+++ b/src/udev/udev-builtin-usb_id.c
@@ -28,6 +28,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include "udev.h"
 
@@ -153,15 +154,15 @@ static int dev_if_packed_info(struct udev_device *dev, 
char *ifs_str, size_t len
 int pos = 0;
 unsigned strpos = 0;
 struct usb_interface_descriptor {
-u_int8_tbLength;
-u_int8_tbDescriptorType;
-u_int8_tbInterfaceNumber;
-u_int8_tbAlternateSetting;
-u_int8_tbNumEndpoints;
-u_int8_tbInterfaceClass;
-u_int8_tbInterfaceSubClass;
-u_int8_tbInterfaceProtocol;
-u_int8_tiInterface;
+uint8_tbLength;
+uint8_tbDescriptorType;
+uint8_tbInterfaceNumber;
+uint8_tbAlternateSetting;
+uint8_tbNumEndpoints;
+uint8_tbInterfaceClass;
+uint8_tbInterfaceSubClass;
+uint8_tbInterfaceProtocol;
+uint8_tiInterface;
 } __attribute__((packed));
 
 if (asprintf(&filename, "%s/descriptors", 
udev_device_get_syspath(dev)) < 0)
@@ -179,21 +180,21 @@ static int dev_if_packed_info(struct udev_device *dev, 
char *ifs_str, size_t len
 
 ifs_str[0] = '\0';
 while (pos < size && strpos+7 < len-2) {
-struct usb_interface_descriptor *desc;
+struct usb_interface_descriptor desc;
 char if_str[8];
 
-desc = (struct usb_interface_descriptor *) &buf[pos];
-if (desc->bLength < 3)
+memcpy(&desc, &buf[pos], sizeof(desc));
+if (desc.bLength < 3)
 break;
-pos += desc->bLength;
+pos += desc.bLength;
 
-if (desc->bDescriptorType != USB_DT_INTERFACE)
+if (desc.bDescriptorType != USB_DT_INTERFACE)
 continue;
 
 if (snprintf(if_str, 8, ":%02x%02x%02x",
- desc->bInterfaceClass,
- desc->bInterfaceSubClass,
- desc->bInterfaceProtocol) != 7)
+ desc.bInterfaceClass,
+ desc.bInterfaceSubClass,
+ desc.bInterfaceProtocol) != 7)
 continue;
 
 if (strstr(ifs_str, if_str) != NULL)
-- 
2.2.1.209.g41e5f3a

___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

[systemd-devel] [PATCH] core/socket: Add REMOTE_IP environment variable for Accept=true

[Shawn Landden]

Fix handling of abstract unix domain sockets too.
---
 TODO |  2 --
 man/systemd.socket.xml   |  5 -
 src/core/service.c   | 24 
 src/shared/socket-util.c | 25 +++--
 4 files changed, 47 insertions(+), 9 deletions(-)

diff --git a/TODO b/TODO
index ae32388..780084a 100644
--- a/TODO
+++ b/TODO
@@ -164,8 +164,6 @@ Features:
 * as soon as we have kdbus, and sender timestamps, revisit coalescing multiple 
parallel daemon reloads:
   http://lists.freedesktop.org/archives/systemd-devel/2014-December/025862.html
 
-* set $REMOTE_IP (or $REMOTE_ADDR/$REMOTE_PORT) environment variable when 
doing per-connection socket activation. use format introduced by xinetd or CGI 
for this
-
 * the install state probably shouldn't get confused by generated units, think 
dbus1/kdbus compat!
 
 * in systemctl list-unit-files: show the install value the presets would 
suggest for a service in a third column
diff --git a/man/systemd.socket.xml b/man/systemd.socket.xml
index 3938345..8796d7b 100644
--- a/man/systemd.socket.xml
+++ b/man/systemd.socket.xml
@@ -357,7 +357,10 @@
 daemons designed for usage with
 
inetd8
 to work unmodified with systemd socket
-activation.
+activation.
+For IPv4 and IPv6 connections the REMOTE_IP
+environment variable will be set with remote IP and port seperated by a
+colon (for SOCK_RAW the port is the IP protocol).
   
 
   
diff --git a/src/core/service.c b/src/core/service.c
index a89ff3f..0942072 100644
--- a/src/core/service.c
+++ b/src/core/service.c
@@ -1119,6 +1119,30 @@ static int service_spawn(
 goto fail;
 }
 
+if (s->accept_socket.unit) {
+union sockaddr_union pn;
+socklen_t pnlen = sizeof(pn);
+_cleanup_free_ char *remote_addr = NULL;
+
+r = getpeername(s->socket_fd, &pn.sa, &pnlen);
+if (r < 0) {
+r = -errno;
+goto fail;
+}
+
+if (pn.sa.sun_family == AF_INET ||
+pn.sa.sun_family == AF_INET6) {
+r = sockaddr_pretty(&pn.sa, pnlen, true, &remote_addr);
+if (r < 0)
+goto fail;
+
+if (asprintf(our_env + n_env++, "REMOTE_IP=%s", 
remote_addr) < 0) {
+r = -ENOMEM;
+goto fail;
+}
+}
+}
+
 final_env = strv_env_merge(2, UNIT(s)->manager->environment, our_env, 
NULL);
 if (!final_env) {
 r = -ENOMEM;
diff --git a/src/shared/socket-util.c b/src/shared/socket-util.c
index 74d90fa..dbe2bf7 100644
--- a/src/shared/socket-util.c
+++ b/src/shared/socket-util.c
@@ -522,19 +522,32 @@ int sockaddr_pretty(const struct sockaddr *_sa, socklen_t 
salen, bool translate_
 return -ENOMEM;
 
 } else if (sa->un.sun_path[0] == 0) {
+void *i;
 /* abstract */
 
-/* FIXME: We assume we can print the
- * socket path here and that it hasn't
- * more than one NUL byte. That is
- * actually an invalid assumption */
-
+/* see unix(7), abstract is wierd */
+i = memchr(&sa->un.sun_path + 1, '\0', 
sizeof(sa->un.sun_path) - 1);
+if (i)
+for (i = (char *)i + 1;
+ (char *)i < 
&sa->un.sun_path[sizeof(sa->un.sun_path)];
+ i = (char *)i + 1)
+if (*(char *)i != '\0') {
+p = strdup("");
+if (!p)
+return -ENOMEM;
+
+goto end;
+}
+
+/* no non-NUL bytes after second NUL (if any) */
 p = new(char, sizeof(sa->un.sun_path)+1);
 if (!p)
 return -ENOMEM;
 
 p[0] = '@';
 memcpy(p+1, sa->un.sun_path+1, 
sizeof(sa->un.sun_path)-1);
+
+/* make printable if there was no second NUL (i == 
NULL) */
 p[sizeof(sa->un.sun_path)] = 0;
 
 } else {
@@ -549,7 +562,7 @@ int sockaddr_pretty(const struct sockaddr *_sa, socklen_t 
salen, bool translate_
 return -ENOTSUP;
 }
 
-
+end:
 *ret = p;
 return 0;
 }
-- 
2.2.1.209.g41e5f3a

___

[systemd-devel] [PATCH] core/socket: Add REMOTE_IP environment variable for Accept=true

[Shawn Landden]

Fix handling of abstract unix domain sockets too.

v2
---
 TODO |  2 --
 man/systemd.socket.xml   |  5 -
 src/core/service.c   | 24 
 src/shared/socket-util.c | 25 +++--
 4 files changed, 47 insertions(+), 9 deletions(-)

diff --git a/TODO b/TODO
index ae32388..780084a 100644
--- a/TODO
+++ b/TODO
@@ -164,8 +164,6 @@ Features:
 * as soon as we have kdbus, and sender timestamps, revisit coalescing multiple 
parallel daemon reloads:
   http://lists.freedesktop.org/archives/systemd-devel/2014-December/025862.html
 
-* set $REMOTE_IP (or $REMOTE_ADDR/$REMOTE_PORT) environment variable when 
doing per-connection socket activation. use format introduced by xinetd or CGI 
for this
-
 * the install state probably shouldn't get confused by generated units, think 
dbus1/kdbus compat!
 
 * in systemctl list-unit-files: show the install value the presets would 
suggest for a service in a third column
diff --git a/man/systemd.socket.xml b/man/systemd.socket.xml
index 3938345..8796d7b 100644
--- a/man/systemd.socket.xml
+++ b/man/systemd.socket.xml
@@ -357,7 +357,10 @@
 daemons designed for usage with
 
inetd8
 to work unmodified with systemd socket
-activation.
+activation.
+For IPv4 and IPv6 connections the REMOTE_IP
+environment variable will be set with remote IP and port seperated by a
+colon (for SOCK_RAW the port is the IP protocol).
   
 
   
diff --git a/src/core/service.c b/src/core/service.c
index a89ff3f..2ee4e11 100644
--- a/src/core/service.c
+++ b/src/core/service.c
@@ -1119,6 +1119,30 @@ static int service_spawn(
 goto fail;
 }
 
+if (s->accept_socket.unit) {
+union sockaddr_union pn;
+socklen_t pnlen = sizeof(pn);
+_cleanup_free_ char *remote_addr = NULL;
+
+r = getpeername(s->socket_fd, &pn.sa, &pnlen);
+if (r < 0) {
+r = -errno;
+goto fail;
+}
+
+if (pn.in.sin_family == AF_INET ||
+pn.in.sin_family == AF_INET6) {
+r = sockaddr_pretty(&pn.sa, pnlen, true, &remote_addr);
+if (r < 0)
+goto fail;
+
+if (asprintf(our_env + n_env++, "REMOTE_IP=%s", 
remote_addr) < 0) {
+r = -ENOMEM;
+goto fail;
+}
+}
+}
+
 final_env = strv_env_merge(2, UNIT(s)->manager->environment, our_env, 
NULL);
 if (!final_env) {
 r = -ENOMEM;
diff --git a/src/shared/socket-util.c b/src/shared/socket-util.c
index 74d90fa..dbe2bf7 100644
--- a/src/shared/socket-util.c
+++ b/src/shared/socket-util.c
@@ -522,19 +522,32 @@ int sockaddr_pretty(const struct sockaddr *_sa, socklen_t 
salen, bool translate_
 return -ENOMEM;
 
 } else if (sa->un.sun_path[0] == 0) {
+void *i;
 /* abstract */
 
-/* FIXME: We assume we can print the
- * socket path here and that it hasn't
- * more than one NUL byte. That is
- * actually an invalid assumption */
-
+/* see unix(7), abstract is wierd */
+i = memchr(&sa->un.sun_path + 1, '\0', 
sizeof(sa->un.sun_path) - 1);
+if (i)
+for (i = (char *)i + 1;
+ (char *)i < 
&sa->un.sun_path[sizeof(sa->un.sun_path)];
+ i = (char *)i + 1)
+if (*(char *)i != '\0') {
+p = strdup("");
+if (!p)
+return -ENOMEM;
+
+goto end;
+}
+
+/* no non-NUL bytes after second NUL (if any) */
 p = new(char, sizeof(sa->un.sun_path)+1);
 if (!p)
 return -ENOMEM;
 
 p[0] = '@';
 memcpy(p+1, sa->un.sun_path+1, 
sizeof(sa->un.sun_path)-1);
+
+/* make printable if there was no second NUL (i == 
NULL) */
 p[sizeof(sa->un.sun_path)] = 0;
 
 } else {
@@ -549,7 +562,7 @@ int sockaddr_pretty(const struct sockaddr *_sa, socklen_t 
salen, bool translate_
 return -ENOTSUP;
 }
 
-
+end:
 *ret = p;
 return 0;
 }
-- 
2.2.1.209.g41e5f3a

___

Re: [systemd-devel] [PATCH] fix strict aliasing violations in src/udev/udev-builtin-usb_id.c

[Shawn Landden]

On Wed, Mar 4, 2015 at 7:58 PM, Zbigniew Jędrzejewski-Szmek <
zbys...@in.waw.pl> wrote:

> On Tue, Mar 03, 2015 at 04:21:30PM -0800, Shawn Landden wrote:
> > also switch to 
> > ---
> >  src/udev/udev-builtin-usb_id.c | 35 ++-
> >  1 file changed, 18 insertions(+), 17 deletions(-)
> >
> > diff --git a/src/udev/udev-builtin-usb_id.c
> b/src/udev/udev-builtin-usb_id.c
> > index ab0d96e..0223421 100644
> > --- a/src/udev/udev-builtin-usb_id.c
> > +++ b/src/udev/udev-builtin-usb_id.c
> > @@ -28,6 +28,7 @@
> >  #include 
> >  #include 
> >  #include 
> > +#include 
> >
> >  #include "udev.h"
> >
> > @@ -153,15 +154,15 @@ static int dev_if_packed_info(struct udev_device
> *dev, char *ifs_str, size_t len
> >  int pos = 0;
> >  unsigned strpos = 0;
> >  struct usb_interface_descriptor {
> > -u_int8_tbLength;
> > -u_int8_tbDescriptorType;
> > -u_int8_tbInterfaceNumber;
> > -u_int8_tbAlternateSetting;
> > -u_int8_tbNumEndpoints;
> > -u_int8_tbInterfaceClass;
> > -u_int8_tbInterfaceSubClass;
> > -u_int8_tbInterfaceProtocol;
> > -u_int8_tiInterface;
> > +uint8_tbLength;
> > +uint8_tbDescriptorType;
> > +uint8_tbInterfaceNumber;
> > +uint8_tbAlternateSetting;
> > +uint8_tbNumEndpoints;
> > +uint8_tbInterfaceClass;
> > +uint8_tbInterfaceSubClass;
> > +uint8_tbInterfaceProtocol;
> > +uint8_tiInterface;
> >  } __attribute__((packed));
> >
> >  if (asprintf(&filename, "%s/descriptors",
> udev_device_get_syspath(dev)) < 0)
> > @@ -179,21 +180,21 @@ static int dev_if_packed_info(struct udev_device
> *dev, char *ifs_str, size_t len
> >
> >  ifs_str[0] = '\0';
> >  while (pos < size && strpos+7 < len-2) {
> > -struct usb_interface_descriptor *desc;
> > +struct usb_interface_descriptor desc;
> >  char if_str[8];
> >
> > -desc = (struct usb_interface_descriptor *) &buf[pos];
> > -if (desc->bLength < 3)
> > +memcpy(&desc, &buf[pos], sizeof(desc));
> Copying it seems suboptimal. But is this actually an aliasing
> violation? buf is a char array, and [1] says: "a character type
> may alias any other type".
>
> Common misunderstanding about strict aliasing. if accessing as char * yes,
but not the other way around. See the C11 standard which makes it clear
(don't have page number off the top of my head...)

> [1]
> https://gcc.gnu.org/onlinedocs/gcc-4.6.1/gcc/Optimize-Options.html#index-fstrict_002daliasing-825
>
> Zbyszek
> ___
> systemd-devel mailing list
> systemd-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/systemd-devel
>



-- 
Shawn Landden
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [PATCH] fix strict aliasing violations in src/udev/udev-builtin-usb_id.c

[Shawn Landden]

Oh wait, I c, yes I had same question.

On Wed, Mar 4, 2015 at 8:07 PM, Shawn Landden  wrote:

> On Wed, Mar 4, 2015 at 7:58 PM, Zbigniew Jędrzejewski-Szmek <
> zbys...@in.waw.pl> wrote:
>
>> On Tue, Mar 03, 2015 at 04:21:30PM -0800, Shawn Landden wrote:
>> > also switch to 
>> > ---
>> >  src/udev/udev-builtin-usb_id.c | 35 ++-
>> >  1 file changed, 18 insertions(+), 17 deletions(-)
>> >
>> > diff --git a/src/udev/udev-builtin-usb_id.c
>> b/src/udev/udev-builtin-usb_id.c
>> > index ab0d96e..0223421 100644
>> > --- a/src/udev/udev-builtin-usb_id.c
>> > +++ b/src/udev/udev-builtin-usb_id.c
>> > @@ -28,6 +28,7 @@
>> >  #include 
>> >  #include 
>> >  #include 
>> > +#include 
>> >
>> >  #include "udev.h"
>> >
>> > @@ -153,15 +154,15 @@ static int dev_if_packed_info(struct udev_device
>> *dev, char *ifs_str, size_t len
>> >  int pos = 0;
>> >  unsigned strpos = 0;
>> >  struct usb_interface_descriptor {
>> > -u_int8_tbLength;
>> > -u_int8_tbDescriptorType;
>> > -u_int8_tbInterfaceNumber;
>> > -u_int8_tbAlternateSetting;
>> > -u_int8_tbNumEndpoints;
>> > -u_int8_tbInterfaceClass;
>> > -u_int8_tbInterfaceSubClass;
>> > -u_int8_tbInterfaceProtocol;
>> > -u_int8_tiInterface;
>> > +uint8_tbLength;
>> > +uint8_tbDescriptorType;
>> > +uint8_tbInterfaceNumber;
>> > +uint8_tbAlternateSetting;
>> > +uint8_tbNumEndpoints;
>> > +uint8_tbInterfaceClass;
>> > +uint8_tbInterfaceSubClass;
>> > +uint8_tbInterfaceProtocol;
>> > +uint8_tiInterface;
>> >  } __attribute__((packed));
>> >
>> >  if (asprintf(&filename, "%s/descriptors",
>> udev_device_get_syspath(dev)) < 0)
>> > @@ -179,21 +180,21 @@ static int dev_if_packed_info(struct udev_device
>> *dev, char *ifs_str, size_t len
>> >
>> >  ifs_str[0] = '\0';
>> >  while (pos < size && strpos+7 < len-2) {
>> > -struct usb_interface_descriptor *desc;
>> > +struct usb_interface_descriptor desc;
>> >  char if_str[8];
>> >
>> > -desc = (struct usb_interface_descriptor *) &buf[pos];
>> > -if (desc->bLength < 3)
>> > +memcpy(&desc, &buf[pos], sizeof(desc));
>> Copying it seems suboptimal. But is this actually an aliasing
>> violation? buf is a char array, and [1] says: "a character type
>> may alias any other type".
>>
>> Common misunderstanding about strict aliasing. if accessing as char *
> yes, but not the other way around. See the C11 standard which makes it
> clear (don't have page number off the top of my head...)
>
>> [1]
>> https://gcc.gnu.org/onlinedocs/gcc-4.6.1/gcc/Optimize-Options.html#index-fstrict_002daliasing-825
>>
>> Zbyszek
>> ___
>> systemd-devel mailing list
>> systemd-devel@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/systemd-devel
>>
>
>
>
> --
> Shawn Landden
>



-- 
Shawn Landden
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [PATCH] core/socket: Add REMOTE_IP environment variable for Accept=true

[Shawn Landden]

On Thu, Mar 5, 2015 at 3:18 AM, Lennart Poettering 
wrote:

> On Wed, 04.03.15 15:18, Shawn Landden (sh...@churchofgit.com) wrote:
>
> Can't this just use getpeername_pretty()?
>
> Then I can't force it to only ipv4 and ipv6.

> Lennart
>
> --
> Lennart Poettering, Red Hat
> ___
> systemd-devel mailing list
> systemd-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/systemd-devel
>



-- 
Shawn Landden
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

[systemd-devel] [PATCH] adjust for time spent in timedated even without dbus timestamp

[Shawn Landden]

it is trivial to fall back to our own timestamp
---
 src/timedate/timedated.c | 9 -
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/src/timedate/timedated.c b/src/timedate/timedated.c
index 88d57e9..7e47348 100644
--- a/src/timedate/timedated.c
+++ b/src/timedate/timedated.c
@@ -551,6 +551,13 @@ static int method_set_time(sd_bus *bus, sd_bus_message *m, 
void *userdata, sd_bu
 if (c->use_ntp)
 return sd_bus_error_setf(error, 
BUS_ERROR_AUTOMATIC_TIME_SYNC_ENABLED, "Automatic time synchronization is 
enabled");
 
+/* this only gets used if dbus does not provide a timestamp
+ * (and ts gets overriden below) */
+r = clock_gettime(CLOCK_MONOTONIC, &ts);
+if (r < 0)
+return -errno;
+start = timespec_load(&ts);
+
 r = sd_bus_message_read(m, "xbb", &utc, &relative, &interactive);
 if (r < 0)
 return r;
@@ -592,7 +599,7 @@ static int method_set_time(sd_bus *bus, sd_bus_message *m, 
void *userdata, sd_bu
 r = sd_bus_message_get_monotonic_usec(m, &start);
 if (r < 0 && r != -ENODATA)
 return r;
-if (r >= 0)
+if (r >= 0 || r == -ENODATA)
 timespec_store(&ts, timespec_load(&ts) + (now(CLOCK_MONOTONIC) 
- start));
 
 /* Set system clock */
-- 
2.2.1.209.g41e5f3a

___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

[systemd-devel] [PATCH] adjust for time spent in timedated even without dbus timestamp

[Shawn Landden]

it is trivial to fall back to our own timestamp

v2: use now()
---
 src/timedate/timedated.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/src/timedate/timedated.c b/src/timedate/timedated.c
index 88d57e9..75b1f1b 100644
--- a/src/timedate/timedated.c
+++ b/src/timedate/timedated.c
@@ -551,6 +551,9 @@ static int method_set_time(sd_bus *bus, sd_bus_message *m, 
void *userdata, sd_bu
 if (c->use_ntp)
 return sd_bus_error_setf(error, 
BUS_ERROR_AUTOMATIC_TIME_SYNC_ENABLED, "Automatic time synchronization is 
enabled");
 
+/* this only gets used if dbus does not provide a timestamp */
+start = now(CLOCK_MONOTONIC);
+
 r = sd_bus_message_read(m, "xbb", &utc, &relative, &interactive);
 if (r < 0)
 return r;
@@ -592,7 +595,7 @@ static int method_set_time(sd_bus *bus, sd_bus_message *m, 
void *userdata, sd_bu
 r = sd_bus_message_get_monotonic_usec(m, &start);
 if (r < 0 && r != -ENODATA)
 return r;
-if (r >= 0)
+if (r >= 0 || r == -ENODATA)
 timespec_store(&ts, timespec_load(&ts) + (now(CLOCK_MONOTONIC) 
- start));
 
 /* Set system clock */
-- 
2.2.1.209.g41e5f3a

___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

[systemd-devel] [PATCH] adjust for time spent in timedated even without dbus timestamp

[Shawn Landden]

it is trivial to fall back to our own timestamp

v2: use now()
v3: remove useless if ()
---
 src/timedate/timedated.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/src/timedate/timedated.c b/src/timedate/timedated.c
index 88d57e9..b8c586c 100644
--- a/src/timedate/timedated.c
+++ b/src/timedate/timedated.c
@@ -551,6 +551,9 @@ static int method_set_time(sd_bus *bus, sd_bus_message *m, 
void *userdata, sd_bu
 if (c->use_ntp)
 return sd_bus_error_setf(error, 
BUS_ERROR_AUTOMATIC_TIME_SYNC_ENABLED, "Automatic time synchronization is 
enabled");
 
+/* this only gets used if dbus does not provide a timestamp */
+start = now(CLOCK_MONOTONIC);
+
 r = sd_bus_message_read(m, "xbb", &utc, &relative, &interactive);
 if (r < 0)
 return r;
@@ -592,7 +595,7 @@ static int method_set_time(sd_bus *bus, sd_bus_message *m, 
void *userdata, sd_bu
 r = sd_bus_message_get_monotonic_usec(m, &start);
 if (r < 0 && r != -ENODATA)
 return r;
-if (r >= 0)
+else
 timespec_store(&ts, timespec_load(&ts) + (now(CLOCK_MONOTONIC) 
- start));
 
 /* Set system clock */
-- 
2.2.1.209.g41e5f3a

___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

[systemd-devel] [PATCH] adjust for time spent in timedated even without dbus timestamp

[Shawn Landden]

it is trivial to fall back to our own timestamp

v2: use now()
v3: remove useless if ()
v4: add comment
---
 src/timedate/timedated.c | 8 ++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/src/timedate/timedated.c b/src/timedate/timedated.c
index 88d57e9..97b535f 100644
--- a/src/timedate/timedated.c
+++ b/src/timedate/timedated.c
@@ -551,6 +551,9 @@ static int method_set_time(sd_bus *bus, sd_bus_message *m, 
void *userdata, sd_bu
 if (c->use_ntp)
 return sd_bus_error_setf(error, 
BUS_ERROR_AUTOMATIC_TIME_SYNC_ENABLED, "Automatic time synchronization is 
enabled");
 
+/* this only gets used if dbus does not provide a timestamp */
+start = now(CLOCK_MONOTONIC);
+
 r = sd_bus_message_read(m, "xbb", &utc, &relative, &interactive);
 if (r < 0)
 return r;
@@ -590,10 +593,11 @@ static int method_set_time(sd_bus *bus, sd_bus_message 
*m, void *userdata, sd_bu
 
 /* adjust ts for time spent in program */
 r = sd_bus_message_get_monotonic_usec(m, &start);
+/* when sd_bus_message_get_monotonic_usec() returns -ENODATA it does 
not modify &start */
 if (r < 0 && r != -ENODATA)
 return r;
-if (r >= 0)
-timespec_store(&ts, timespec_load(&ts) + (now(CLOCK_MONOTONIC) 
- start));
+
+timespec_store(&ts, timespec_load(&ts) + (now(CLOCK_MONOTONIC) - 
start));
 
 /* Set system clock */
 if (clock_settime(CLOCK_REALTIME, &ts) < 0) {
-- 
2.2.1.209.g41e5f3a

___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

[systemd-devel] [PATCH] add REMOTE_ADDR and REMOTE_PORT for Accept=yes

[Shawn Landden]

---
 TODO   |  2 --
 man/systemd.socket.xml |  6 +-
 src/core/service.c | 47 +++
 3 files changed, 52 insertions(+), 3 deletions(-)

diff --git a/TODO b/TODO
index ae32388..780084a 100644
--- a/TODO
+++ b/TODO
@@ -164,8 +164,6 @@ Features:
 * as soon as we have kdbus, and sender timestamps, revisit coalescing multiple 
parallel daemon reloads:
   http://lists.freedesktop.org/archives/systemd-devel/2014-December/025862.html
 
-* set $REMOTE_IP (or $REMOTE_ADDR/$REMOTE_PORT) environment variable when 
doing per-connection socket activation. use format introduced by xinetd or CGI 
for this
-
 * the install state probably shouldn't get confused by generated units, think 
dbus1/kdbus compat!
 
 * in systemctl list-unit-files: show the install value the presets would 
suggest for a service in a third column
diff --git a/man/systemd.socket.xml b/man/systemd.socket.xml
index 3938345..20f1e0c 100644
--- a/man/systemd.socket.xml
+++ b/man/systemd.socket.xml
@@ -357,7 +357,11 @@
 daemons designed for usage with
 
inetd8
 to work unmodified with systemd socket
-activation.
+activation.
+For IPv4 and IPv6 connections the REMOTE_ADDR
+environment variable will be set with remote IP, and 
REMOTE_PORT
+environment variable set to the remote port, similar to CGI
+(for SOCK_RAW the port is the IP protocol).
   
 
   
diff --git a/src/core/service.c b/src/core/service.c
index cc4ea19..6a690ac 100644
--- a/src/core/service.c
+++ b/src/core/service.c
@@ -22,6 +22,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include "async.h"
 #include "manager.h"
@@ -1119,6 +1120,52 @@ static int service_spawn(
 goto fail;
 }
 
+if (s->accept_socket.unit) {
+union sockaddr_union sa;
+socklen_t salen = sizeof(sa);
+_cleanup_free_ char *remote_addr = NULL;
+char a[MAX(INET6_ADDRSTRLEN, INET_ADDRSTRLEN)];
+
+r = getpeername(s->socket_fd, &sa.sa, &salen);
+if (r < 0) {
+r = -errno;
+goto fail;
+}
+
+if (sa.sa.sa_family == AF_INET ||
+sa.sa.sa_family == AF_INET6) {
+if (inet_ntop(sa.sa.sa_family,
+  /* this field of the API is kinda 
braindead,
+   * should take head of struct so it can 
be passed the union...*/
+  sa.sa.sa_family == AF_INET6 ?
+&sa.in6.sin6_addr :
+&sa.in.sin_addr,
+  a, sizeof(a)) == NULL) {
+r = -errno;
+goto fail;
+}
+
+if (asprintf(our_env + n_env++,
+ "REMOTE_ADDR=%s",
+ /* musl and glibc inet_ntop() present 
v4-mapped addresses in :::a.b.c.d form */
+ sa.sa.sa_family == AF_INET6 && strchr(a, 
'.') ?
+   strempty(startswith(a, ":::")) :
+   a) < 0) {
+r = -ENOMEM;
+goto fail;
+}
+
+if (asprintf(our_env + n_env++,
+ "REMOTE_PORT=%u",
+ ntohs(sa.sa.sa_family == AF_INET6 ?
+ sa.in6.sin6_port :
+ sa.in.sin_port)) < 0) {
+r = -ENOMEM;
+goto fail;
+}
+}
+}
+
 final_env = strv_env_merge(2, UNIT(s)->manager->environment, our_env, 
NULL);
 if (!final_env) {
 r = -ENOMEM;
-- 
2.2.1.209.g41e5f3a

___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [PATCH] add REMOTE_ADDR and REMOTE_PORT for Accept=yes

[Shawn Landden]

the xinetd REMOTE_IP is a fedora extension so I think we should avoid it.

On Sun, Mar 8, 2015 at 4:24 PM, Shawn Landden  wrote:

> ---
>  TODO   |  2 --
>  man/systemd.socket.xml |  6 +-
>  src/core/service.c | 47
> +++
>  3 files changed, 52 insertions(+), 3 deletions(-)
>
> diff --git a/TODO b/TODO
> index ae32388..780084a 100644
> --- a/TODO
> +++ b/TODO
> @@ -164,8 +164,6 @@ Features:
>  * as soon as we have kdbus, and sender timestamps, revisit coalescing
> multiple parallel daemon reloads:
>
> http://lists.freedesktop.org/archives/systemd-devel/2014-December/025862.html
>
> -* set $REMOTE_IP (or $REMOTE_ADDR/$REMOTE_PORT) environment variable when
> doing per-connection socket activation. use format introduced by xinetd or
> CGI for this
> -
>  * the install state probably shouldn't get confused by generated units,
> think dbus1/kdbus compat!
>
>  * in systemctl list-unit-files: show the install value the presets would
> suggest for a service in a third column
> diff --git a/man/systemd.socket.xml b/man/systemd.socket.xml
> index 3938345..20f1e0c 100644
> --- a/man/systemd.socket.xml
> +++ b/man/systemd.socket.xml
> @@ -357,7 +357,11 @@
>  daemons designed for usage with
>
>  
> inetd8
>  to work unmodified with systemd socket
> -activation.
> +activation.
> +For IPv4 and IPv6 connections the
> REMOTE_ADDR
> +environment variable will be set with remote IP, and
> REMOTE_PORT
> +environment variable set to the remote port, similar to CGI
> +(for SOCK_RAW the port is the IP protocol).
>
>
>
> diff --git a/src/core/service.c b/src/core/service.c
> index cc4ea19..6a690ac 100644
> --- a/src/core/service.c
> +++ b/src/core/service.c
> @@ -22,6 +22,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>
>  #include "async.h"
>  #include "manager.h"
> @@ -1119,6 +1120,52 @@ static int service_spawn(
>  goto fail;
>  }
>
> +if (s->accept_socket.unit) {
> +union sockaddr_union sa;
> +socklen_t salen = sizeof(sa);
> +_cleanup_free_ char *remote_addr = NULL;
> +char a[MAX(INET6_ADDRSTRLEN, INET_ADDRSTRLEN)];
> +
> +r = getpeername(s->socket_fd, &sa.sa, &salen);
> +if (r < 0) {
> +r = -errno;
> +goto fail;
> +}
> +
> +if (sa.sa.sa_family == AF_INET ||
> +sa.sa.sa_family == AF_INET6) {
> +if (inet_ntop(sa.sa.sa_family,
> +  /* this field of the API is kinda
> braindead,
> +   * should take head of struct so it
> can be passed the union...*/
> +  sa.sa.sa_family == AF_INET6 ?
> +&sa.in6.sin6_addr :
> +&sa.in.sin_addr,
> +  a, sizeof(a)) == NULL) {
> +r = -errno;
> +goto fail;
> +}
> +
> +if (asprintf(our_env + n_env++,
> + "REMOTE_ADDR=%s",
> + /* musl and glibc inet_ntop()
> present v4-mapped addresses in :::a.b.c.d form */
> + sa.sa.sa_family == AF_INET6 &&
> strchr(a, '.') ?
> +   strempty(startswith(a, ":::"))
> :
> +   a) < 0) {
> +r = -ENOMEM;
> +goto fail;
> +}
> +
> +if (asprintf(our_env + n_env++,
> + "REMOTE_PORT=%u",
> + ntohs(sa.sa.sa_family == AF_INET6 ?
> + sa.in6.sin6_port :
> + sa.in.sin_port)) < 0) {
> +r = -ENOMEM;
> +goto fail;
> +}
> +}
> +}
> +
>  final_env = strv_env_merge(2, UNIT(s)->manager->environment,
> our_env, NULL);
>  if (!final_env) {
>  r = -ENOMEM;
> --
> 2.2.1.209.g41e5f3a
>
>


-- 
Shawn Landden
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [PATCH] fix strict aliasing violations in src/udev/udev-builtin-usb_id.c

[Shawn Landden]

On Sun, Mar 8, 2015 at 4:11 PM, Lennart Poettering 
wrote:

> On Thu, 05.03.15 04:58, Zbigniew Jędrzejewski-Szmek (zbys...@in.waw.pl)
> wrote:
>
> > > +uint8_tbLength;
> > > +uint8_tbDescriptorType;
> > > +uint8_tbInterfaceNumber;
> > > +uint8_tbAlternateSetting;
> > > +uint8_tbNumEndpoints;
> > > +uint8_tbInterfaceClass;
> > > +uint8_tbInterfaceSubClass;
> > > +uint8_tbInterfaceProtocol;
> > > +uint8_tiInterface;
> > >  } __attribute__((packed));
> > >
> > >  if (asprintf(&filename, "%s/descriptors",
> udev_device_get_syspath(dev)) < 0)
> > > @@ -179,21 +180,21 @@ static int dev_if_packed_info(struct udev_device
> *dev, char *ifs_str, size_t len
> > >
> > >  ifs_str[0] = '\0';
> > >  while (pos < size && strpos+7 < len-2) {
> > > -struct usb_interface_descriptor *desc;
> > > +struct usb_interface_descriptor desc;
> > >  char if_str[8];
> > >
> > > -desc = (struct usb_interface_descriptor *) &buf[pos];
> > > -if (desc->bLength < 3)
> > > +memcpy(&desc, &buf[pos], sizeof(desc));
> > Copying it seems suboptimal. But is this actually an aliasing
> > violation? buf is a char array, and [1] says: "a character type
> > may alias any other type".
> >
> > [1]
> https://gcc.gnu.org/onlinedocs/gcc-4.6.1/gcc/Optimize-Options.html#index-fstrict_002daliasing-825
>
> Also, I greatly prefer using unions for these things, to make the
> aliasing explicit, rather than copying things.
>
> The other solution I had was to use offsetof() to basically make it an
enum, but that made the code was quite a bit more verbose.

> Lennart
>
> --
> Lennart Poettering, Red Hat
> ___
> systemd-devel mailing list
> systemd-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/systemd-devel
>



-- 
Shawn Landden
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [PATCH] add REMOTE_ADDR and REMOTE_PORT for Accept=yes

[Shawn Landden]

On Mon, Mar 9, 2015 at 9:22 AM, Lennart Poettering 
wrote:

> On Sun, 08.03.15 16:24, Shawn Landden (sh...@churchofgit.com) wrote:
>
> >
> > diff --git a/src/core/service.c b/src/core/service.c
> > index cc4ea19..6a690ac 100644
> > --- a/src/core/service.c
> > +++ b/src/core/service.c
> > @@ -22,6 +22,7 @@
> >  #include 
> >  #include 
> >  #include 
> > +#include 
> >
> >  #include "async.h"
> >  #include "manager.h"
> > @@ -1119,6 +1120,52 @@ static int service_spawn(
> >  goto fail;
> >  }
> >
> > +if (s->accept_socket.unit) {
>
> Please use UNIT_DEREF() for this.
>
> > +union sockaddr_union sa;
> > +socklen_t salen = sizeof(sa);
> > +_cleanup_free_ char *remote_addr = NULL;
> > +char a[MAX(INET6_ADDRSTRLEN, INET_ADDRSTRLEN)];
> > +
> > +r = getpeername(s->socket_fd, &sa.sa, &salen);
> > +if (r < 0) {
> > +r = -errno;
> > +goto fail;
> > +}
> > +
> > +if (sa.sa.sa_family == AF_INET ||
> > +sa.sa.sa_family == AF_INET6) {
> > +if (inet_ntop(sa.sa.sa_family,
> > +  /* this field of the API is kinda
> braindead,
> > +   * should take head of struct so
> it can be passed the union...*/
> > +  sa.sa.sa_family == AF_INET6 ?
> > +&sa.in6.sin6_addr :
> > +&sa.in.sin_addr,
> > +  a, sizeof(a)) == NULL) {
> > +r = -errno;
> > +goto fail;
> > +}
>
> Hmm, so we already have sockaddr_pretty() in socket-util.c. Can't we
> fold this logic into that? Maybe add a boolean param that indicates
> whether to append the port number or not.
>
> I'd rather not have code for this all over the place.
>
> > +
> > +if (asprintf(our_env + n_env++,
> > + "REMOTE_ADDR=%s",
> > + /* musl and glibc inet_ntop()
> present v4-mapped addresses in :::a.b.c.d form */
> > + sa.sa.sa_family == AF_INET6 &&
> strchr(a, '.') ?
> > +   strempty(startswith(a,
> ":::")) :
> > +   a) < 0) {
> > +r = -ENOMEM;
> > +goto fail;
> > +}
>
> sockaddr_pretty() already has propery code for this, please use
> that. Also, we don't care about non-glibc libcs anyway, hence please
> no reference to that.
>
How about doing it this way in sockaddr_pretty() instead of rewriting
inet_ntop() to the fact that the man page does not say that glibc does this?
Otherwise I agree with everything in this review.

>
> Then, we try to avoid asprintf() if we just want to concat strings,
> please use strappend() or strjoin() for that.
>
> > +
> > +if (asprintf(our_env + n_env++,
> > + "REMOTE_PORT=%u",
> > + ntohs(sa.sa.sa_family == AF_INET6 ?
> > + sa.in6.sin6_port :
> > + sa.in.sin_port)) < 0) {
> > +r = -ENOMEM;
> > +goto fail;
>
>
> To make this easy I think a new sockaddr_port() call in socket-util.c
> would make sense that returns an "int", in which it either encodes an
> error or the converted port number.
>
> Lennart
>
> --
> Lennart Poettering, Red Hat
> ___
> systemd-devel mailing list
> systemd-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/systemd-devel
>



-- 
Shawn Landden
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

[systemd-devel] [PATCH] add REMOTE_ADDR and REMOTE_PORT for Accept=yes

[Shawn Landden]

---
 TODO |  2 -
 man/systemd.socket.xml   |  6 ++-
 src/core/service.c   | 35 +-
 src/libsystemd/sd-resolve/test-resolve.c |  2 +-
 src/shared/socket-util.c | 80 ++--
 src/shared/socket-util.h |  4 +-
 src/timesync/timesyncd-server.h  |  2 +-
 7 files changed, 88 insertions(+), 43 deletions(-)

diff --git a/TODO b/TODO
index ae32388..780084a 100644
--- a/TODO
+++ b/TODO
@@ -164,8 +164,6 @@ Features:
 * as soon as we have kdbus, and sender timestamps, revisit coalescing multiple 
parallel daemon reloads:
   http://lists.freedesktop.org/archives/systemd-devel/2014-December/025862.html
 
-* set $REMOTE_IP (or $REMOTE_ADDR/$REMOTE_PORT) environment variable when 
doing per-connection socket activation. use format introduced by xinetd or CGI 
for this
-
 * the install state probably shouldn't get confused by generated units, think 
dbus1/kdbus compat!
 
 * in systemctl list-unit-files: show the install value the presets would 
suggest for a service in a third column
diff --git a/man/systemd.socket.xml b/man/systemd.socket.xml
index 3938345..20f1e0c 100644
--- a/man/systemd.socket.xml
+++ b/man/systemd.socket.xml
@@ -357,7 +357,11 @@
 daemons designed for usage with
 
inetd8
 to work unmodified with systemd socket
-activation.
+activation.
+For IPv4 and IPv6 connections the REMOTE_ADDR
+environment variable will be set with remote IP, and 
REMOTE_PORT
+environment variable set to the remote port, similar to CGI
+(for SOCK_RAW the port is the IP protocol).
   
 
   
diff --git a/src/core/service.c b/src/core/service.c
index cc4ea19..89feec4 100644
--- a/src/core/service.c
+++ b/src/core/service.c
@@ -1095,7 +1095,7 @@ static int service_spawn(
 if (r < 0)
 goto fail;
 
-our_env = new0(char*, 4);
+our_env = new0(char*, 6);
 if (!our_env) {
 r = -ENOMEM;
 goto fail;
@@ -1119,6 +1119,39 @@ static int service_spawn(
 goto fail;
 }
 
+if (UNIT_DEREF(s->accept_socket)) {
+union sockaddr_union sa;
+socklen_t salen = sizeof(sa);
+
+r = getpeername(s->socket_fd, &sa.sa, &salen);
+if (r < 0) {
+r = -errno;
+goto fail;
+}
+
+if (sa.sa.sa_family == AF_INET ||
+sa.sa.sa_family == AF_INET6) {
+_cleanup_free_ char *addr = NULL;
+uint16_t port = (uint16_t)sockaddr_port(&sa);
+
+r = sockaddr_pretty(&sa.sa, salen, true, false, &addr);
+if (r < 0)
+goto fail;
+
+if (!(our_env[n_env++] = strappend("REMOTE_ADDR=", 
addr))) {
+r = -ENOMEM;
+goto fail;
+}
+
+if (asprintf(our_env + n_env++,
+ "REMOTE_PORT=%u",
+ port) < 0) {
+r = -ENOMEM;
+goto fail;
+}
+}
+}
+
 final_env = strv_env_merge(2, UNIT(s)->manager->environment, our_env, 
NULL);
 if (!final_env) {
 r = -ENOMEM;
diff --git a/src/libsystemd/sd-resolve/test-resolve.c 
b/src/libsystemd/sd-resolve/test-resolve.c
index 3187ce9..354a407 100644
--- a/src/libsystemd/sd-resolve/test-resolve.c
+++ b/src/libsystemd/sd-resolve/test-resolve.c
@@ -46,7 +46,7 @@ static int getaddrinfo_handler(sd_resolve_query *q, int ret, 
const struct addrin
 for (i = ai; i; i = i->ai_next) {
 _cleanup_free_ char *addr = NULL;
 
-assert_se(sockaddr_pretty(i->ai_addr, i->ai_addrlen, false, 
&addr) == 0);
+assert_se(sockaddr_pretty(i->ai_addr, i->ai_addrlen, false, 
true, &addr) == 0);
 puts(addr);
 }
 
diff --git a/src/shared/socket-util.c b/src/shared/socket-util.c
index 74d90fa..d7d34f8 100644
--- a/src/shared/socket-util.c
+++ b/src/shared/socket-util.c
@@ -297,7 +297,7 @@ int socket_address_print(const SocketAddress *a, char 
**ret) {
 return 0;
 }
 
-return sockaddr_pretty(&a->sockaddr.sa, a->size, false, ret);
+return sockaddr_pretty(&a->sockaddr.sa, a->size, false, true, ret);
 }
 
 bool socket_address_can_accept(const SocketAddress *a) {
@@ -466,7 +466,17 @@ bool socket_address_matches_fd(const SocketAddress *a, int 
fd) {
 return socket_address_equal(a, &b);
 }
 
-int sockaddr_pretty(const struct sockaddr *_sa, socklen_t salen, bool 
translate_ipv6, char **ret) {
+int sockaddr_port(const u

Re: [systemd-devel] [PATCH] add REMOTE_ADDR and REMOTE_PORT for Accept=yes

[Shawn Landden]

On Mon, Mar 9, 2015 at 1:18 PM, Lennart Poettering 
wrote:

> On Mon, 09.03.15 13:09, Shawn Landden (sh...@churchofgit.com) wrote:
>
> > +if (UNIT_DEREF(s->accept_socket)) {
> > +union sockaddr_union sa;
> > +socklen_t salen = sizeof(sa);
> > +
> > +r = getpeername(s->socket_fd, &sa.sa, &salen);
> > +if (r < 0) {
> > +r = -errno;
> > +goto fail;
> > +}
> > +
> > +if (sa.sa.sa_family == AF_INET ||
> > +sa.sa.sa_family == AF_INET6) {
> > +_cleanup_free_ char *addr = NULL;
> > +uint16_t port = (uint16_t)sockaddr_port(&sa);
>
> We try to avoid invoking functions in the same lines as we declare
> variables.
>
> Also, even though this cannot fail in this case, I think it would be
> nicer, to make port an int, and check for < 0 and return an error in
> that case...
>
> > +
> > +r = sockaddr_pretty(&sa.sa, salen, true,
> false, &addr);
> > +if (r < 0)
> > +goto fail;
> > +
> > +if (!(our_env[n_env++] =
> strappend("REMOTE_ADDR=", addr))) {
>
> In newer code we try to avoid making assignments and doing if checks
> in the same line.
>
> >
> > -int sockaddr_pretty(const struct sockaddr *_sa, socklen_t salen, bool
> translate_ipv6, char **ret) {
> > +int sockaddr_port(const union sockaddr_union *sa) {
> > +assert_return(sa->sa.sa_family == AF_INET6 ||
> > +  sa->sa.sa_family == AF_INET,
> > +  -ENOTSUP);
>
> assert_return() is a macro to use only for cases where there's a clear
> programming error of the caller. But I am pretty sure that this is not
> the case here. If it gets called on an non-AF_INET/AF_INET6 socket
> then this should fail without logging.
>
> Also I think an error code of EAFNOSUPPORT would be more appropriate.
>
> Hmm, and I think this should probably take an actual "struct
> sockaddr", rather than a union sockaddr_union...
>
> > @@ -475,42 +485,40 @@ int sockaddr_pretty(const struct sockaddr *_sa,
> socklen_t salen, bool translate_
> >
> >  switch (sa->sa.sa_family) {
> >
> > -case AF_INET: {
> > -uint32_t a;
> > -
> > -a = ntohl(sa->in.sin_addr.s_addr);
> > -
> > -if (asprintf(&p,
> > - "%u.%u.%u.%u:%u",
> > - a >> 24, (a >> 16) & 0xFF, (a >> 8) &
> 0xFF, a & 0xFF,
> > - ntohs(sa->in.sin_port)) < 0)
> > -return -ENOMEM;
> > -
> > -break;
> > -}
> > -
> > +case AF_INET:
> >  case AF_INET6: {
> > -static const unsigned char ipv4_prefix[] = {
> > -0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0xFF, 0xFF
> > -};
> > -
> > -if (translate_ipv6 && memcmp(&sa->in6.sin6_addr,
> ipv4_prefix, sizeof(ipv4_prefix)) == 0) {
> > -const uint8_t *a = sa->in6.sin6_addr.s6_addr+12;
> > +char a[MAX(INET6_ADDRSTRLEN, INET_ADDRSTRLEN)];
> > +const char *addr;
> > +bool ipv4_mapped = false;
> > +
> > +if (inet_ntop(sa->sa.sa_family,
> > +  /* this field of the API is kinda
> braindead,
> > +   * should take head of struct so it can
> be passed the union...*/
> > +  sa->sa.sa_family == AF_INET6 ?
> > +&sa->in6.sin6_addr :
> > +&sa->in.sin_addr,
> > +  a, sizeof(a)) == NULL)
> > +  return -ENOMEM;
> > +
> > +/* glibc inet_ntop() presents v4-mapped addresses in
> :::a.b.c.d form */
> > +if (translate_ipv6 && sa->sa.sa_family == AF_INET6 &&
> strchr(a, '.')) {
> > +ipv4_mapped = true;
> > +addr = strempty(startswith(a, ":::"));
>
> I think it would be a lot nicer to check the raw IP prefix as before,
> and only the

Re: [systemd-devel] [PATCH] add REMOTE_ADDR and REMOTE_PORT for Accept=yes

[Shawn Landden]

On Mon, Mar 9, 2015 at 1:18 PM, Lennart Poettering 
wrote:

> On Mon, 09.03.15 13:09, Shawn Landden (sh...@churchofgit.com) wrote:
>
> > +if (UNIT_DEREF(s->accept_socket)) {
> > +union sockaddr_union sa;
> > +socklen_t salen = sizeof(sa);
> > +
> > +r = getpeername(s->socket_fd, &sa.sa, &salen);
> > +if (r < 0) {
> > +r = -errno;
> > +goto fail;
> > +}
> > +
> > +if (sa.sa.sa_family == AF_INET ||
> > +sa.sa.sa_family == AF_INET6) {
> > +_cleanup_free_ char *addr = NULL;
> > +uint16_t port = (uint16_t)sockaddr_port(&sa);
>
> We try to avoid invoking functions in the same lines as we declare
> variables.
>
> Also, even though this cannot fail in this case, I think it would be
> nicer, to make port an int, and check for < 0 and return an error in
> that case...
>
> > +
> > +r = sockaddr_pretty(&sa.sa, salen, true,
> false, &addr);
> > +if (r < 0)
> > +goto fail;
> > +
> > +if (!(our_env[n_env++] =
> strappend("REMOTE_ADDR=", addr))) {
>
> In newer code we try to avoid making assignments and doing if checks
> in the same line.
>
> Taking this out would be pretty gruesome because of use of the
post-increment operator.

> >
> > -int sockaddr_pretty(const struct sockaddr *_sa, socklen_t salen, bool
> translate_ipv6, char **ret) {
> > +int sockaddr_port(const union sockaddr_union *sa) {
> > +assert_return(sa->sa.sa_family == AF_INET6 ||
> > +  sa->sa.sa_family == AF_INET,
> > +  -ENOTSUP);
>
> assert_return() is a macro to use only for cases where there's a clear
> programming error of the caller. But I am pretty sure that this is not
> the case here. If it gets called on an non-AF_INET/AF_INET6 socket
> then this should fail without logging.
>
> Also I think an error code of EAFNOSUPPORT would be more appropriate.
>
> Hmm, and I think this should probably take an actual "struct
> sockaddr", rather than a union sockaddr_union...
>
> > @@ -475,42 +485,40 @@ int sockaddr_pretty(const struct sockaddr *_sa,
> socklen_t salen, bool translate_
> >
> >  switch (sa->sa.sa_family) {
> >
> > -case AF_INET: {
> > -uint32_t a;
> > -
> > -a = ntohl(sa->in.sin_addr.s_addr);
> > -
> > -if (asprintf(&p,
> > - "%u.%u.%u.%u:%u",
> > - a >> 24, (a >> 16) & 0xFF, (a >> 8) &
> 0xFF, a & 0xFF,
> > - ntohs(sa->in.sin_port)) < 0)
> > -return -ENOMEM;
> > -
> > -break;
> > -}
> > -
> > +case AF_INET:
> >  case AF_INET6: {
> > -static const unsigned char ipv4_prefix[] = {
> > -0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0xFF, 0xFF
> > -};
> > -
> > -if (translate_ipv6 && memcmp(&sa->in6.sin6_addr,
> ipv4_prefix, sizeof(ipv4_prefix)) == 0) {
> > -const uint8_t *a = sa->in6.sin6_addr.s6_addr+12;
> > +char a[MAX(INET6_ADDRSTRLEN, INET_ADDRSTRLEN)];
> > +const char *addr;
> > +bool ipv4_mapped = false;
> > +
> > +if (inet_ntop(sa->sa.sa_family,
> > +  /* this field of the API is kinda
> braindead,
> > +   * should take head of struct so it can
> be passed the union...*/
> > +  sa->sa.sa_family == AF_INET6 ?
> > +&sa->in6.sin6_addr :
> > +&sa->in.sin_addr,
> > +  a, sizeof(a)) == NULL)
> > +  return -ENOMEM;
> > +
> > +/* glibc inet_ntop() presents v4-mapped addresses in
> :::a.b.c.d form */
> > +if (translate_ipv6 && sa->sa.sa_family == AF_INET6 &&
> strchr(a, '.')) {
> > +ipv4_mapped = true;
> > +addr = st

[systemd-devel] [PATCH] add REMOTE_ADDR and REMOTE_PORT for Accept=yes

[Shawn Landden]

---
 TODO |  2 -
 man/systemd.socket.xml   |  6 ++-
 src/core/service.c   | 39 +++-
 src/libsystemd/sd-resolve/test-resolve.c |  2 +-
 src/shared/socket-util.c | 76 +++-
 src/shared/socket-util.h |  4 +-
 src/timesync/timesyncd-server.h  |  2 +-
 7 files changed, 103 insertions(+), 28 deletions(-)

diff --git a/TODO b/TODO
index ae32388..780084a 100644
--- a/TODO
+++ b/TODO
@@ -164,8 +164,6 @@ Features:
 * as soon as we have kdbus, and sender timestamps, revisit coalescing multiple 
parallel daemon reloads:
   http://lists.freedesktop.org/archives/systemd-devel/2014-December/025862.html
 
-* set $REMOTE_IP (or $REMOTE_ADDR/$REMOTE_PORT) environment variable when 
doing per-connection socket activation. use format introduced by xinetd or CGI 
for this
-
 * the install state probably shouldn't get confused by generated units, think 
dbus1/kdbus compat!
 
 * in systemctl list-unit-files: show the install value the presets would 
suggest for a service in a third column
diff --git a/man/systemd.socket.xml b/man/systemd.socket.xml
index 3938345..20f1e0c 100644
--- a/man/systemd.socket.xml
+++ b/man/systemd.socket.xml
@@ -357,7 +357,11 @@
 daemons designed for usage with
 
inetd8
 to work unmodified with systemd socket
-activation.
+activation.
+For IPv4 and IPv6 connections the REMOTE_ADDR
+environment variable will be set with remote IP, and 
REMOTE_PORT
+environment variable set to the remote port, similar to CGI
+(for SOCK_RAW the port is the IP protocol).
   
 
   
diff --git a/src/core/service.c b/src/core/service.c
index cc4ea19..7067a64 100644
--- a/src/core/service.c
+++ b/src/core/service.c
@@ -1095,7 +1095,7 @@ static int service_spawn(
 if (r < 0)
 goto fail;
 
-our_env = new0(char*, 4);
+our_env = new0(char*, 6);
 if (!our_env) {
 r = -ENOMEM;
 goto fail;
@@ -1119,6 +1119,43 @@ static int service_spawn(
 goto fail;
 }
 
+if (UNIT_DEREF(s->accept_socket)) {
+union sockaddr_union sa;
+socklen_t salen = sizeof(sa);
+
+r = getpeername(s->socket_fd, &sa.sa, &salen);
+if (r < 0) {
+r = -errno;
+goto fail;
+}
+
+if (sa.sa.sa_family == AF_INET ||
+sa.sa.sa_family == AF_INET6) {
+_cleanup_free_ char *addr = NULL;
+int port;
+
+r = sockaddr_pretty(&sa.sa, salen, true, false, &addr);
+if (r < 0)
+goto fail;
+
+if (!(our_env[n_env++] = strappend("REMOTE_ADDR=", 
addr))) {
+r = -ENOMEM;
+goto fail;
+}
+
+port = sockaddr_port(&sa.sa);
+if (port < 0) {
+r = port;
+goto fail;
+}
+
+if (asprintf(our_env[n_env++], "REMOTE_PORT=%u", port) 
< 0) {
+r = -ENOMEM;
+goto fail;
+}
+}
+}
+
 final_env = strv_env_merge(2, UNIT(s)->manager->environment, our_env, 
NULL);
 if (!final_env) {
 r = -ENOMEM;
diff --git a/src/libsystemd/sd-resolve/test-resolve.c 
b/src/libsystemd/sd-resolve/test-resolve.c
index 3187ce9..354a407 100644
--- a/src/libsystemd/sd-resolve/test-resolve.c
+++ b/src/libsystemd/sd-resolve/test-resolve.c
@@ -46,7 +46,7 @@ static int getaddrinfo_handler(sd_resolve_query *q, int ret, 
const struct addrin
 for (i = ai; i; i = i->ai_next) {
 _cleanup_free_ char *addr = NULL;
 
-assert_se(sockaddr_pretty(i->ai_addr, i->ai_addrlen, false, 
&addr) == 0);
+assert_se(sockaddr_pretty(i->ai_addr, i->ai_addrlen, false, 
true, &addr) == 0);
 puts(addr);
 }
 
diff --git a/src/shared/socket-util.c b/src/shared/socket-util.c
index 74d90fa..ca8ff39 100644
--- a/src/shared/socket-util.c
+++ b/src/shared/socket-util.c
@@ -297,7 +297,7 @@ int socket_address_print(const SocketAddress *a, char 
**ret) {
 return 0;
 }
 
-return sockaddr_pretty(&a->sockaddr.sa, a->size, false, ret);
+return sockaddr_pretty(&a->sockaddr.sa, a->size, false, true, ret);
 }
 
 bool socket_address_can_accept(const SocketAddress *a) {
@@ -466,7 +466,21 @@ bool socket_address_matches_fd(const SocketAddress *a, int 
fd) {
 return socket_address_equal(a, &b);
 }
 
-int sockaddr_pretty(cons

[systemd-devel] [PATCH] add REMOTE_ADDR and REMOTE_PORT for Accept=yes

[Shawn Landden]

---
 TODO |  2 -
 man/systemd.socket.xml   |  7 ++-
 src/core/service.c   | 41 -
 src/libsystemd/sd-resolve/test-resolve.c |  2 +-
 src/shared/socket-util.c | 76 +++-
 src/shared/socket-util.h |  4 +-
 src/timesync/timesyncd-server.h  |  2 +-
 7 files changed, 106 insertions(+), 28 deletions(-)

diff --git a/TODO b/TODO
index ae32388..780084a 100644
--- a/TODO
+++ b/TODO
@@ -164,8 +164,6 @@ Features:
 * as soon as we have kdbus, and sender timestamps, revisit coalescing multiple 
parallel daemon reloads:
   http://lists.freedesktop.org/archives/systemd-devel/2014-December/025862.html
 
-* set $REMOTE_IP (or $REMOTE_ADDR/$REMOTE_PORT) environment variable when 
doing per-connection socket activation. use format introduced by xinetd or CGI 
for this
-
 * the install state probably shouldn't get confused by generated units, think 
dbus1/kdbus compat!
 
 * in systemctl list-unit-files: show the install value the presets would 
suggest for a service in a third column
diff --git a/man/systemd.socket.xml b/man/systemd.socket.xml
index 3938345..6808179 100644
--- a/man/systemd.socket.xml
+++ b/man/systemd.socket.xml
@@ -357,7 +357,12 @@
 daemons designed for usage with
 
inetd8
 to work unmodified with systemd socket
-activation.
+activation.
+
+For IPv4 and IPv6 connections the REMOTE_ADDR
+environment variable will contain the remote IP, and 
REMOTE_PORT
+will contain the remote port. This is the same as the format used by 
CGI.
+For SOCK_RAW the port is the IP protocol.
   
 
   
diff --git a/src/core/service.c b/src/core/service.c
index cc4ea19..bcfce96 100644
--- a/src/core/service.c
+++ b/src/core/service.c
@@ -1095,7 +1095,7 @@ static int service_spawn(
 if (r < 0)
 goto fail;
 
-our_env = new0(char*, 4);
+our_env = new0(char*, 6);
 if (!our_env) {
 r = -ENOMEM;
 goto fail;
@@ -1119,6 +1119,45 @@ static int service_spawn(
 goto fail;
 }
 
+if (UNIT_DEREF(s->accept_socket)) {
+union sockaddr_union sa;
+socklen_t salen = sizeof(sa);
+
+r = getpeername(s->socket_fd, &sa.sa, &salen);
+if (r < 0) {
+r = -errno;
+goto fail;
+}
+
+if (IN_SET(sa.sa.sa_family, AF_INET, AF_INET6)) {
+_cleanup_free_ char *addr = NULL;
+char *t;
+int port;
+
+r = sockaddr_pretty(&sa.sa, salen, true, false, &addr);
+if (r < 0)
+goto fail;
+
+t = strappend("REMOTE_ADDR=", addr);
+if (!t) {
+r = -ENOMEM;
+goto fail;
+}
+our_env[n_env++] = t;
+
+port = sockaddr_port(&sa.sa);
+if (port < 0) {
+r = port;
+goto fail;
+}
+
+if (asprintf((our_env + n_env++), "REMOTE_PORT=%u", 
port) < 0) {
+r = -ENOMEM;
+goto fail;
+}
+}
+}
+
 final_env = strv_env_merge(2, UNIT(s)->manager->environment, our_env, 
NULL);
 if (!final_env) {
 r = -ENOMEM;
diff --git a/src/libsystemd/sd-resolve/test-resolve.c 
b/src/libsystemd/sd-resolve/test-resolve.c
index 3187ce9..354a407 100644
--- a/src/libsystemd/sd-resolve/test-resolve.c
+++ b/src/libsystemd/sd-resolve/test-resolve.c
@@ -46,7 +46,7 @@ static int getaddrinfo_handler(sd_resolve_query *q, int ret, 
const struct addrin
 for (i = ai; i; i = i->ai_next) {
 _cleanup_free_ char *addr = NULL;
 
-assert_se(sockaddr_pretty(i->ai_addr, i->ai_addrlen, false, 
&addr) == 0);
+assert_se(sockaddr_pretty(i->ai_addr, i->ai_addrlen, false, 
true, &addr) == 0);
 puts(addr);
 }
 
diff --git a/src/shared/socket-util.c b/src/shared/socket-util.c
index 74d90fa..0d87cb1 100644
--- a/src/shared/socket-util.c
+++ b/src/shared/socket-util.c
@@ -297,7 +297,7 @@ int socket_address_print(const SocketAddress *a, char 
**ret) {
 return 0;
 }
 
-return sockaddr_pretty(&a->sockaddr.sa, a->size, false, ret);
+return sockaddr_pretty(&a->sockaddr.sa, a->size, false, true, ret);
 }
 
 bool socket_address_can_accept(const SocketAddress *a) {
@@ -466,7 +466,20 @@ bool socket_address_matches_fd(const SocketAddress *a, int 
fd) {
 

[systemd-devel] [PATCH] fix strict aliasing violations in src/udev/udev-builtin-usb_id.c

[Shawn Landden]

also switch to 
---
 src/udev/udev-builtin-usb_id.c | 34 +-
 1 file changed, 17 insertions(+), 17 deletions(-)

diff --git a/src/udev/udev-builtin-usb_id.c b/src/udev/udev-builtin-usb_id.c
index ab0d96e..b42b32e 100644
--- a/src/udev/udev-builtin-usb_id.c
+++ b/src/udev/udev-builtin-usb_id.c
@@ -28,6 +28,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include "udev.h"
 
@@ -153,15 +154,15 @@ static int dev_if_packed_info(struct udev_device *dev, 
char *ifs_str, size_t len
 int pos = 0;
 unsigned strpos = 0;
 struct usb_interface_descriptor {
-u_int8_tbLength;
-u_int8_tbDescriptorType;
-u_int8_tbInterfaceNumber;
-u_int8_tbAlternateSetting;
-u_int8_tbNumEndpoints;
-u_int8_tbInterfaceClass;
-u_int8_tbInterfaceSubClass;
-u_int8_tbInterfaceProtocol;
-u_int8_tiInterface;
+uint8_tbLength;
+uint8_tbDescriptorType;
+uint8_tbInterfaceNumber;
+uint8_tbAlternateSetting;
+uint8_tbNumEndpoints;
+uint8_tbInterfaceClass;
+uint8_tbInterfaceSubClass;
+uint8_tbInterfaceProtocol;
+uint8_tiInterface;
 } __attribute__((packed));
 
 if (asprintf(&filename, "%s/descriptors", 
udev_device_get_syspath(dev)) < 0)
@@ -179,21 +180,20 @@ static int dev_if_packed_info(struct udev_device *dev, 
char *ifs_str, size_t len
 
 ifs_str[0] = '\0';
 while (pos < size && strpos+7 < len-2) {
-struct usb_interface_descriptor *desc;
+char *desc = &buf[pos];
 char if_str[8];
 
-desc = (struct usb_interface_descriptor *) &buf[pos];
-if (desc->bLength < 3)
+if (desc[offsetof(struct usb_interface_descriptor, bLength)] < 
3)
 break;
-pos += desc->bLength;
+pos += desc[offsetof(struct usb_interface_descriptor, 
bLength)];
 
-if (desc->bDescriptorType != USB_DT_INTERFACE)
+if (desc[offsetof(struct usb_interface_descriptor, 
bDescriptorType)] != USB_DT_INTERFACE)
 continue;
 
 if (snprintf(if_str, 8, ":%02x%02x%02x",
- desc->bInterfaceClass,
- desc->bInterfaceSubClass,
- desc->bInterfaceProtocol) != 7)
+ desc[offsetof(struct usb_interface_descriptor, 
bInterfaceClass)],
+ desc[offsetof(struct usb_interface_descriptor, 
bInterfaceSubClass)],
+ desc[offsetof(struct usb_interface_descriptor, 
bInterfaceProtocol)]) != 7)
 continue;
 
 if (strstr(ifs_str, if_str) != NULL)
-- 
2.2.1.209.g41e5f3a

___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

[systemd-devel] [PATCH] fix compiler warning

[Shawn Landden]

warning: pointer/integer type mismatch in conditional expression
---
 src/shared/socket-util.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/shared/socket-util.c b/src/shared/socket-util.c
index 5820279..73e1177 100644
--- a/src/shared/socket-util.c
+++ b/src/shared/socket-util.c
@@ -475,8 +475,8 @@ int sockaddr_port(const struct sockaddr *_sa) {
 return -EAFNOSUPPORT;
 
 return ntohs(sa->sa.sa_family == AF_INET6 ?
-   sa->in6.sin6_port :
-   sa->in.sin_port);
+   (uint16_t)sa->in6.sin6_port :
+   (uint16_t)sa->in.sin_port);
 }
 
 int sockaddr_pretty(const struct sockaddr *_sa, socklen_t salen, bool 
translate_ipv6, bool include_port, char **ret) {
-- 
2.2.1.209.g41e5f3a

___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

[systemd-devel] [PATCH] fix strict aliasing issue in src/libsystemd-network/sd-dhcp-client.c

[Shawn Landden]

---
 src/libsystemd-network/sd-dhcp-client.c | 11 +++
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/src/libsystemd-network/sd-dhcp-client.c 
b/src/libsystemd-network/sd-dhcp-client.c
index 4224e01..a477ccc 100644
--- a/src/libsystemd-network/sd-dhcp-client.c
+++ b/src/libsystemd-network/sd-dhcp-client.c
@@ -1469,7 +1469,7 @@ static int client_receive_message_udp(sd_event_source *s, 
int fd,
 _cleanup_free_ DHCPMessage *message = NULL;
 int buflen = 0, len, r;
 const struct ether_addr zero_mac = { { 0, 0, 0, 0, 0, 0 } };
-const struct ether_addr *expected_chaddr = NULL;
+bool expect_chaddr;
 uint8_t expected_hlen = 0;
 
 assert(s);
@@ -1514,11 +1514,11 @@ static int client_receive_message_udp(sd_event_source 
*s, int fd,
 
 if (client->arp_type == ARPHRD_ETHER) {
 expected_hlen = ETH_ALEN;
-expected_chaddr = (const struct ether_addr *) 
&client->mac_addr;
+expect_chaddr = true;
 } else {
/* Non-ethernet links expect zero chaddr */
expected_hlen = 0;
-   expected_chaddr = &zero_mac;
+   expect_chaddr = false;
 }
 
 if (message->hlen != expected_hlen) {
@@ -1526,7 +1526,10 @@ static int client_receive_message_udp(sd_event_source 
*s, int fd,
 return 0;
 }
 
-if (memcmp(&message->chaddr[0], expected_chaddr, ETH_ALEN)) {
+if (memcmp(&message->chaddr[0], expect_chaddr ?
+  (void *)&client->mac_addr :
+  (void *)&zero_mac,
+ETH_ALEN)) {
 log_dhcp_client(client, "received chaddr does not match "
 "expected: ignoring");
 return 0;
-- 
2.2.1.209.g41e5f3a

___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

[systemd-devel] [PATCH] fix strict aliasing issues in src/udev/udev-ctrl.c

[Shawn Landden]

it is ironic that
"The only purpose of this structure is to cast the structure pointer
passed in addr in order to avoid compiler warnings.  See EXAMPLE below."
from bind(2)
---
 src/udev/udev-ctrl.c | 19 ++-
 1 file changed, 10 insertions(+), 9 deletions(-)

diff --git a/src/udev/udev-ctrl.c b/src/udev/udev-ctrl.c
index c0c5981..61d3c5b 100644
--- a/src/udev/udev-ctrl.c
+++ b/src/udev/udev-ctrl.c
@@ -18,6 +18,7 @@
 #include 
 #include 
 
+#include "socket-util.h"
 #include "udev.h"
 
 /* wire protocol magic must match */
@@ -55,7 +56,7 @@ struct udev_ctrl {
 int refcount;
 struct udev *udev;
 int sock;
-struct sockaddr_un saddr;
+union sockaddr_union saddr;
 socklen_t addrlen;
 bool bound;
 bool cleanup_socket;
@@ -94,9 +95,9 @@ struct udev_ctrl *udev_ctrl_new_from_fd(struct udev *udev, 
int fd) {
 if (r < 0)
 log_warning_errno(errno, "could not set SO_PASSCRED: %m");
 
-uctrl->saddr.sun_family = AF_LOCAL;
-strscpy(uctrl->saddr.sun_path, sizeof(uctrl->saddr.sun_path), 
"/run/udev/control");
-uctrl->addrlen = offsetof(struct sockaddr_un, sun_path) + 
strlen(uctrl->saddr.sun_path);
+uctrl->saddr.un.sun_family = AF_LOCAL;
+strscpy(uctrl->saddr.un.sun_path, sizeof(uctrl->saddr.un.sun_path), 
"/run/udev/control");
+uctrl->addrlen = offsetof(struct sockaddr_un, sun_path) + 
strlen(uctrl->saddr.un.sun_path);
 return uctrl;
 }
 
@@ -108,10 +109,10 @@ int udev_ctrl_enable_receiving(struct udev_ctrl *uctrl) {
 int err;
 
 if (!uctrl->bound) {
-err = bind(uctrl->sock, (struct sockaddr *)&uctrl->saddr, 
uctrl->addrlen);
+err = bind(uctrl->sock, &uctrl->saddr.sa, uctrl->addrlen);
 if (err < 0 && errno == EADDRINUSE) {
-unlink(uctrl->saddr.sun_path);
-err = bind(uctrl->sock, (struct sockaddr 
*)&uctrl->saddr, uctrl->addrlen);
+unlink(uctrl->saddr.un.sun_path);
+err = bind(uctrl->sock, &uctrl->saddr.sa, 
uctrl->addrlen);
 }
 
 if (err < 0) {
@@ -160,7 +161,7 @@ int udev_ctrl_cleanup(struct udev_ctrl *uctrl) {
 if (uctrl == NULL)
 return 0;
 if (uctrl->cleanup_socket)
-unlink(uctrl->saddr.sun_path);
+unlink(uctrl->saddr.un.sun_path);
 return 0;
 }
 
@@ -249,7 +250,7 @@ static int ctrl_send(struct udev_ctrl *uctrl, enum 
udev_ctrl_msg_type type, int
 ctrl_msg_wire.intval = intval;
 
 if (!uctrl->connected) {
-if (connect(uctrl->sock, (struct sockaddr *)&uctrl->saddr, 
uctrl->addrlen) < 0) {
+if (connect(uctrl->sock, &uctrl->saddr.sa, uctrl->addrlen) < 
0) {
 err = -errno;
 goto out;
 }
-- 
2.2.1.209.g41e5f3a

___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

[systemd-devel] [PATCH 5/6] network: fix strict aliasing issue

[Shawn Landden]

We shouldn't assume 64-bit arch with the way we do math either.
(although I will submit a patch to glibc to add a uint64_t union alias)
---
 src/network/networkd-address.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/src/network/networkd-address.c b/src/network/networkd-address.c
index 0be6165..4b7f451 100644
--- a/src/network/networkd-address.c
+++ b/src/network/networkd-address.c
@@ -605,12 +605,12 @@ bool address_equal(Address *a1, Address *a2) {
 }
 
 case AF_INET6: {
-uint64_t *b1, *b2;
+uint32_t *b1, *b2;
 
-b1 = (uint64_t*)&a1->in_addr.in6;
-b2 = (uint64_t*)&a2->in_addr.in6;
+b1 = &a1->in_addr.in6.s6_addr32[0];
+b2 = &a2->in_addr.in6.s6_addr32[0];
 
-return (((b1[0] ^ b2[0]) | (b1[1] ^ b2[1])) == 0UL);
+return (((b1[0] ^ b2[0]) | (b1[1] ^ b2[1]) | (b1[2] ^ b2[2]) | 
(b1[3] ^  b2[3])) == 0);
 }
 
 default:
-- 
2.2.1.209.g41e5f3a

___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

[systemd-devel] [PATCH 1/6] fix strict aliasing issues in src/udev/udev-ctrl.c

[Shawn Landden]

it is ironic that
"The only purpose of this structure is to cast the structure pointer
passed in addr in order to avoid compiler warnings.  See EXAMPLE below."
from bind(2)
---
 src/udev/udev-ctrl.c | 19 ++-
 1 file changed, 10 insertions(+), 9 deletions(-)

diff --git a/src/udev/udev-ctrl.c b/src/udev/udev-ctrl.c
index c0c5981..61d3c5b 100644
--- a/src/udev/udev-ctrl.c
+++ b/src/udev/udev-ctrl.c
@@ -18,6 +18,7 @@
 #include 
 #include 
 
+#include "socket-util.h"
 #include "udev.h"
 
 /* wire protocol magic must match */
@@ -55,7 +56,7 @@ struct udev_ctrl {
 int refcount;
 struct udev *udev;
 int sock;
-struct sockaddr_un saddr;
+union sockaddr_union saddr;
 socklen_t addrlen;
 bool bound;
 bool cleanup_socket;
@@ -94,9 +95,9 @@ struct udev_ctrl *udev_ctrl_new_from_fd(struct udev *udev, 
int fd) {
 if (r < 0)
 log_warning_errno(errno, "could not set SO_PASSCRED: %m");
 
-uctrl->saddr.sun_family = AF_LOCAL;
-strscpy(uctrl->saddr.sun_path, sizeof(uctrl->saddr.sun_path), 
"/run/udev/control");
-uctrl->addrlen = offsetof(struct sockaddr_un, sun_path) + 
strlen(uctrl->saddr.sun_path);
+uctrl->saddr.un.sun_family = AF_LOCAL;
+strscpy(uctrl->saddr.un.sun_path, sizeof(uctrl->saddr.un.sun_path), 
"/run/udev/control");
+uctrl->addrlen = offsetof(struct sockaddr_un, sun_path) + 
strlen(uctrl->saddr.un.sun_path);
 return uctrl;
 }
 
@@ -108,10 +109,10 @@ int udev_ctrl_enable_receiving(struct udev_ctrl *uctrl) {
 int err;
 
 if (!uctrl->bound) {
-err = bind(uctrl->sock, (struct sockaddr *)&uctrl->saddr, 
uctrl->addrlen);
+err = bind(uctrl->sock, &uctrl->saddr.sa, uctrl->addrlen);
 if (err < 0 && errno == EADDRINUSE) {
-unlink(uctrl->saddr.sun_path);
-err = bind(uctrl->sock, (struct sockaddr 
*)&uctrl->saddr, uctrl->addrlen);
+unlink(uctrl->saddr.un.sun_path);
+err = bind(uctrl->sock, &uctrl->saddr.sa, 
uctrl->addrlen);
 }
 
 if (err < 0) {
@@ -160,7 +161,7 @@ int udev_ctrl_cleanup(struct udev_ctrl *uctrl) {
 if (uctrl == NULL)
 return 0;
 if (uctrl->cleanup_socket)
-unlink(uctrl->saddr.sun_path);
+unlink(uctrl->saddr.un.sun_path);
 return 0;
 }
 
@@ -249,7 +250,7 @@ static int ctrl_send(struct udev_ctrl *uctrl, enum 
udev_ctrl_msg_type type, int
 ctrl_msg_wire.intval = intval;
 
 if (!uctrl->connected) {
-if (connect(uctrl->sock, (struct sockaddr *)&uctrl->saddr, 
uctrl->addrlen) < 0) {
+if (connect(uctrl->sock, &uctrl->saddr.sa, uctrl->addrlen) < 
0) {
 err = -errno;
 goto out;
 }
-- 
2.2.1.209.g41e5f3a

___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

[systemd-devel] [PATCH] shutdown: add kexec loading, avoid calling `kexec` binary unnessecarily

[Shawn Landden]

 if (streq_ptr(a, b)) return 0;
 
-char oldch1, oldch2;
-char abuf[strlen(a)+1], bbuf[strlen(b)+1];
-char *str1 = abuf, *str2 = bbuf;
-char * one, * two;
-int rc;
-int isnum;
-
 strcpy(str1, a);
 strcpy(str2, b);
 
diff --git a/src/shared/strv.c b/src/shared/strv.c
index ee45ad1..ed28b95 100644
--- a/src/shared/strv.c
+++ b/src/shared/strv.c
@@ -81,7 +81,14 @@ void strv_clear(char **l) {
 }
 
 void strv_free(char **l) {
-strv_clear(l);
+char **k;
+
+if (!l)
+return;
+
+for (k = l; *k; k++)
+free(*k);
+
 free(l);
 }
 
diff --git a/src/systemctl/bootspec.c b/src/systemctl/bootspec.c
new file mode 100644
index 000..823e768
--- /dev/null
+++ b/src/systemctl/bootspec.c
@@ -0,0 +1,231 @@
+/*-*- Mode: C; c-basic-offset: 8; indent-tabs-mode: nil -*-*/
+
+/***
+  This file is part of systemd.
+
+  Copyright 2015 Shawn Landden
+
+  systemd is free software; you can redistribute it and/or modify it
+  under the terms of the GNU Lesser General Public License as published by
+  the Free Software Foundation; either version 2.1 of the License, or
+  (at your option) any later version.
+
+  systemd is distributed in the hope that it will be useful, but
+  WITHOUT ANY WARRANTY; without even the implied warranty of
+  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+  Lesser General Public License for more details.
+
+  You should have received a copy of the GNU Lesser General Public License
+  along with systemd; If not, see <http://www.gnu.org/licenses/>.
+***/
+
+/*
+ * Implements http://freedesktop.org/wiki/Specifications/BootLoaderSpec/
+ * for use with kexec
+ */
+
+#include 
+#include 
+#include 
+#include 
+
+#include "bootspec.h"
+#include "strv.h"
+#include "fileio.h"
+#include "rpmvercmp.h"
+
+void bootspec_free(struct BootSpec *s) {
+if (!s)
+return;
+
+free(s->conf);
+free(s);
+}
+
+static int bootspec_cmp(const void *left, const void *right) {
+const struct BootSpec *l = left,
+  *r = right;
+
+/* reverse sort to put highest version first */
+return rpmvercmp(r->version, l->version);
+}
+
+int bootspec_getkernels(struct BootSpec **ret, size_t *cl) {
+int r = 0;
+struct BootSpec *c;
+_cleanup_closedir_ DIR *d;
+struct dirent *dent;
+size_t i = 1, ii = 2;
+_cleanup_close_ int midfd = -1;
+ssize_t ss;
+char mid_char[32 + 1];
+sd_id128_t machine_id;
+
+midfd = open("/etc/machine-id", O_RDONLY);
+if (midfd < 0)
+return -errno;
+ss = read(midfd, &mid_char, sizeof(mid_char));
+if (ss < 0)
+return -errno;
+else if (ss != 33)
+return -EBADF;
+if (mid_char[32] == '\n')
+mid_char[32] = '\0';
+else
+return -EBADF;
+r = sd_id128_from_string(mid_char, &machine_id);
+if (r < 0)
+return r;
+
+c = new0(struct BootSpec, i + 1);
+if (!c)
+return -ENOMEM;
+
+d = opendir(BOOTENTRIESDIR);
+if (!d) {
+if (errno == ENOENT)
+return 0;
+else
+return -errno;
+}
+
+for (dent = readdir(d); dent != NULL; dent = readdir(d)) {
+struct BootSpec *bs;
+char *m, *l, *k;
+_cleanup_fclose_ FILE *f = NULL;
+
+if (!endswith(dent->d_name, ".conf"))
+continue;
+
+c = greedy_realloc0((void **)&c, &ii, i + 2, sizeof(struct 
BootSpec));
+if (!c)
+return -ENOMEM;
+bs = &c[i - 1];
+
+f = fopenat(dirfd(d), dent->d_name, "r");
+if (!f)
+return -errno;
+
+r = read_full_stream(f, &bs->conf, NULL);
+if (r < 0)
+return r;
+
+for (m = bs->conf; ; m = k + 1) {
+if (m[0] == '#')
+continue;
+
+k = strchr(m, '\n');
+
+if (k)
+*k = '\0';
+else
+break;
+
+if  ((l = startswith(m, "title ")))
+bs->title  = l + strspn(l, WHITESPACE);
+else if ((l = startswith(m, "version ")))
+bs->version= l + strspn(l, WHITESPACE);
+else if ((l = startswith(m, "machine-id

[systemd-devel] [PATCH] shutdown: add kexec loading, avoid calling `kexec` binary unnessecarily

[Shawn Landden]

  return -1;
+else
+return 0;
+}
+if (!b)
+return 1;
+
 /* easy comparison to see if versions are identical */
 if (streq_ptr(a, b)) return 0;
 
-char oldch1, oldch2;
-char abuf[strlen(a)+1], bbuf[strlen(b)+1];
-char *str1 = abuf, *str2 = bbuf;
-char * one, * two;
-int rc;
-int isnum;
-
 strcpy(str1, a);
 strcpy(str2, b);
 
diff --git a/src/shared/strv.c b/src/shared/strv.c
index ee45ad1..ed28b95 100644
--- a/src/shared/strv.c
+++ b/src/shared/strv.c
@@ -81,7 +81,14 @@ void strv_clear(char **l) {
 }
 
 void strv_free(char **l) {
-strv_clear(l);
+char **k;
+
+if (!l)
+return;
+
+for (k = l; *k; k++)
+free(*k);
+
 free(l);
 }
 
diff --git a/src/systemctl/bootspec.c b/src/systemctl/bootspec.c
new file mode 100644
index 000..cabd46c
--- /dev/null
+++ b/src/systemctl/bootspec.c
@@ -0,0 +1,231 @@
+/*-*- Mode: C; c-basic-offset: 8; indent-tabs-mode: nil -*-*/
+
+/***
+  This file is part of systemd.
+
+  Copyright 2015 Shawn Landden
+
+  systemd is free software; you can redistribute it and/or modify it
+  under the terms of the GNU Lesser General Public License as published by
+  the Free Software Foundation; either version 2.1 of the License, or
+  (at your option) any later version.
+
+  systemd is distributed in the hope that it will be useful, but
+  WITHOUT ANY WARRANTY; without even the implied warranty of
+  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+  Lesser General Public License for more details.
+
+  You should have received a copy of the GNU Lesser General Public License
+  along with systemd; If not, see <http://www.gnu.org/licenses/>.
+***/
+
+/*
+ * Implements http://freedesktop.org/wiki/Specifications/BootLoaderSpec/
+ * for use with kexec
+ */
+
+#include 
+#include 
+#include 
+#include 
+
+#include "bootspec.h"
+#include "strv.h"
+#include "fileio.h"
+#include "rpmvercmp.h"
+
+void bootspec_free(struct BootSpec *s) {
+if (!s)
+return;
+
+free(s->conf);
+free(s);
+}
+
+static int bootspec_cmp(const void *left, const void *right) {
+const struct BootSpec *l = left,
+  *r = right;
+
+/* reverse sort to put highest version first */
+return rpmvercmp(r->version, l->version);
+}
+
+int bootspec_getkernels(struct BootSpec **ret, size_t *cl) {
+int r = 0;
+struct BootSpec *c;
+_cleanup_closedir_ DIR *d;
+struct dirent *dent;
+size_t i = 1, ii = 2;
+_cleanup_close_ int midfd = -1;
+ssize_t ss;
+char mid_char[32 + 1];
+sd_id128_t machine_id;
+
+midfd = open("/etc/machine-id", O_RDONLY);
+if (midfd < 0)
+return -errno;
+ss = read(midfd, &mid_char, sizeof(mid_char));
+if (ss < 0)
+return -errno;
+else if (ss != 33)
+return -EBADF;
+if (mid_char[32] == '\n')
+mid_char[32] = '\0';
+else
+return -EBADF;
+r = sd_id128_from_string(mid_char, &machine_id);
+if (r < 0)
+return r;
+
+c = new0(struct BootSpec, i + 1);
+if (!c)
+return -ENOMEM;
+
+d = opendir(BOOTENTRIESDIR);
+if (!d) {
+if (errno == ENOENT)
+return 0;
+else
+return -errno;
+}
+
+for (dent = readdir(d); dent != NULL; dent = readdir(d)) {
+struct BootSpec *bs;
+char *m, *l, *k;
+_cleanup_fclose_ FILE *f = NULL;
+
+if (!endswith(dent->d_name, ".conf"))
+continue;
+
+c = greedy_realloc0((void **)&c, &ii, i + 2, sizeof(struct 
BootSpec));
+if (!c)
+return -ENOMEM;
+bs = &c[i - 1];
+
+f = fopenat(dirfd(d), dent->d_name, "r");
+if (!f)
+return -errno;
+
+r = read_full_stream(f, &bs->conf, NULL);
+if (r < 0)
+return r;
+
+for (m = bs->conf; ; m = k + 1) {
+if (m[0] == '#')
+continue;
+
+k = strchr(m, '\n');
+
+if (k)
+*k = '\0';
+else
+break;
+
+if  ((l = startswith(m, "title ")))
+bs->title  = l + strspn(l, WHITESPACE);
+els

[systemd-devel] [PATCH] shutdown: add kexec loading, avoid calling `kexec` binary unnessecarily

[Shawn Landden]

   char abuf[strlen(a)+1], bbuf[strlen(b)+1];
+char *str1 = abuf, *str2 = bbuf;
+char * one, * two;
+int rc;
+int isnum;
+
+if (!a) {
+if (b)
+return -1;
+else
+return 0;
+}
+if (!b)
+return 1;
+
 /* easy comparison to see if versions are identical */
 if (streq_ptr(a, b)) return 0;
 
-char oldch1, oldch2;
-char abuf[strlen(a)+1], bbuf[strlen(b)+1];
-char *str1 = abuf, *str2 = bbuf;
-char * one, * two;
-int rc;
-int isnum;
-
 strcpy(str1, a);
 strcpy(str2, b);
 
diff --git a/src/shared/strv.c b/src/shared/strv.c
index ee45ad1..ed28b95 100644
--- a/src/shared/strv.c
+++ b/src/shared/strv.c
@@ -81,7 +81,14 @@ void strv_clear(char **l) {
 }
 
 void strv_free(char **l) {
-strv_clear(l);
+char **k;
+
+if (!l)
+return;
+
+for (k = l; *k; k++)
+free(*k);
+
 free(l);
 }
 
diff --git a/src/systemctl/bootspec.c b/src/systemctl/bootspec.c
new file mode 100644
index 000..8194eaf
--- /dev/null
+++ b/src/systemctl/bootspec.c
@@ -0,0 +1,247 @@
+/*-*- Mode: C; c-basic-offset: 8; indent-tabs-mode: nil -*-*/
+
+/***
+  This file is part of systemd.
+
+  Copyright 2015 Shawn Landden
+
+  systemd is free software; you can redistribute it and/or modify it
+  under the terms of the GNU Lesser General Public License as published by
+  the Free Software Foundation; either version 2.1 of the License, or
+  (at your option) any later version.
+
+  systemd is distributed in the hope that it will be useful, but
+  WITHOUT ANY WARRANTY; without even the implied warranty of
+  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+  Lesser General Public License for more details.
+
+  You should have received a copy of the GNU Lesser General Public License
+  along with systemd; If not, see <http://www.gnu.org/licenses/>.
+***/
+
+/*
+ * Implements http://freedesktop.org/wiki/Specifications/BootLoaderSpec/
+ * for use with kexec
+ */
+
+#include 
+#include 
+#include 
+#include 
+
+#include "bootspec.h"
+#include "strv.h"
+#include "fileio.h"
+#include "rpmvercmp.h"
+
+void bootspec_free(struct BootSpec *s) {
+if (!s)
+return;
+
+free(s->conf);
+free(s);
+}
+
+static int bootspec_cmp(const void *left, const void *right) {
+const struct BootSpec *l = left,
+  *r = right;
+
+/* reverse sort to put highest version first */
+return rpmvercmp(r->version, l->version);
+}
+
+int bootspec_getkernels(struct BootSpec **ret, size_t *cl, int 
*bootdir_fd_ret) {
+int r = 0;
+struct BootSpec *c;
+_cleanup_closedir_ DIR *d = NULL;
+struct dirent *dent;
+size_t i = 1, ii = 2;
+_cleanup_close_ int midfd = -1;
+int bootdir_fd = -1;
+ssize_t ss;
+char mid_char[32 + 1];
+sd_id128_t machine_id;
+
+assert(ret);
+assert(cl);
+
+/* we assume that the EFI partition is mounted
+ * and at /boot/efi or /boot
+ */
+errno = 0;
+if (access("/boot/efi/loader/entries", R_OK|X_OK) == 0)
+bootdir_fd = open("/boot/efi", O_DIRECTORY);
+else if (access("/boot/loader/entries",   R_OK|X_OK) == 0)
+bootdir_fd = open("/boot", O_DIRECTORY);
+if (bootdir_fd < 0)
+return IN_SET(errno, 0, ENOENT) ? 0 : -errno;
+
+midfd = open("/etc/machine-id", O_RDONLY);
+if (midfd < 0)
+return -errno;
+ss = read(midfd, &mid_char, sizeof(mid_char));
+if (ss < 0)
+return -errno;
+else if (ss != 33)
+return -EBADF;
+if (mid_char[32] == '\n')
+mid_char[32] = '\0';
+else
+return -EBADF;
+r = sd_id128_from_string(mid_char, &machine_id);
+if (r < 0)
+return r;
+
+c = new0(struct BootSpec, i + 1);
+if (!c)
+return -ENOMEM;
+
+d = opendirat(bootdir_fd, "loader/entries");
+if (!d) {
+if (errno == ENOENT)
+return 0;
+else
+return -errno;
+}
+
+for (dent = readdir(d); dent != NULL; dent = readdir(d)) {
+struct BootSpec *bs;
+char *m, *l, *k;
+_cleanup_fclose_ FILE *f = NULL;
+
+if (!endswith(dent->d_name, ".conf"))
+continue;
+
+c = greedy_realloc0((void **)&c, &ii, i + 2, sizeof(struct 
BootSpec));
+if (!c)

Re: [systemd-devel] [PATCH] shutdown: add kexec loading, avoid calling `kexec` binary unnessecarily

[Shawn Landden]

On Wed, Mar 11, 2015 at 5:51 PM, Kay Sievers  wrote:

> On Thu, Mar 12, 2015 at 1:22 AM, Shawn Landden 
> wrote:
> > Still use helper when Xen Dom0, to avoid duplicating some hairy code.
> >
> > I think the rbtree version was far more understandable as
> greedy_realloc0()
> > is very messy to do correctly.
> >
> > Take fopenat() from lsof.
> > Add opendirat()
>
> We have that in util.c :: xopendirat()
>
> > Future: generate BootLoaderSpec files for other kernel install locations
>
> This approach duplicates, the potentially complex, boot manager kernel
> selection logic.
>
> The recent systemd-boot boot loader and efi stub loader which carries
> the kernel, the cmdline, the initrd in one single EFI binary will also
> not use any boot loader snippets, it will be discovered by the loader
> itself, which parses the PE/COFF files and looks for specific content.
>
> The snippets are meant to unify the boot loader *configuration*, but
> they do not mean that every bootable kernel will or should have one.
> There might be many ways for kernels to be found by the boot loader,
> the snippets are just one source for that.
>
> I'm not sure what exact problem this patch tries to solve,

rebooting with kexec is faster than a full reboot. Currently we do not
support kexec very well. Lennart asked for something like this, but perhaps
we no longer want to support kexec loading?

> but it
> generally does not sound right to duplicate the boot loader
> selection/discovery/enumeration logic here. I don't really think we
> should do that, or will be able to catch with the boot loader up here.
>
> Thanks,
> Kay
> ___
> systemd-devel mailing list
> systemd-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/systemd-devel
>



-- 
Shawn Landden
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [PATCH] shutdown: add kexec loading, avoid calling `kexec` binary unnessecarily

[Shawn Landden]

On Wed, Mar 11, 2015 at 5:51 PM, Kay Sievers  wrote:

> On Thu, Mar 12, 2015 at 1:22 AM, Shawn Landden 
> wrote:
> > Still use helper when Xen Dom0, to avoid duplicating some hairy code.
> >
> > I think the rbtree version was far more understandable as
> greedy_realloc0()
> > is very messy to do correctly.
> >
> > Take fopenat() from lsof.
> > Add opendirat()
>
> We have that in util.c :: xopendirat()
>
> > Future: generate BootLoaderSpec files for other kernel install locations
>
> This approach duplicates, the potentially complex, boot manager kernel
> selection logic.
>
>
Hopefully it could be unified in some way. I find it disturbing that
systemd will have a great deal of EFI-specific code.

> The recent systemd-boot boot loader and efi stub loader which carries
> the kernel, the cmdline, the initrd in one single EFI binary will also
> not use any boot loader snippets, it will be discovered by the loader
> itself, which parses the PE/COFF files and looks for specific content.
>
> The snippets are meant to unify the boot loader *configuration*, but
> they do not mean that every bootable kernel will or should have one.
> There might be many ways for kernels to be found by the boot loader,
> the snippets are just one source for that.
>
> I'm not sure what exact problem this patch tries to solve, but it
> generally does not sound right to duplicate the boot loader
> selection/discovery/enumeration logic here. I don't really think we
> should do that, or will be able to catch with the boot loader up here.
>
> Thanks,
> Kay
> ___
> systemd-devel mailing list
> systemd-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/systemd-devel
>



-- 
Shawn Landden
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [PATCH] shutdown: add kexec loading, avoid calling `kexec` binary unnessecarily

[Shawn Landden]

On Wed, Mar 11, 2015 at 6:24 PM, Kay Sievers  wrote:

> On Thu, Mar 12, 2015 at 2:07 AM, Shawn Landden 
> wrote:
> > On Wed, Mar 11, 2015 at 5:51 PM, Kay Sievers  wrote:
> >>
> >> On Thu, Mar 12, 2015 at 1:22 AM, Shawn Landden 
> >> wrote:
> >> > Still use helper when Xen Dom0, to avoid duplicating some hairy code.
> >> >
> >> > I think the rbtree version was far more understandable as
> >> > greedy_realloc0()
> >> > is very messy to do correctly.
> >> >
> >> > Take fopenat() from lsof.
> >> > Add opendirat()
> >>
> >> We have that in util.c :: xopendirat()
> >>
> >> > Future: generate BootLoaderSpec files for other kernel install
> locations
> >>
> >> This approach duplicates, the potentially complex, boot manager kernel
> >> selection logic.
> >>
> >> The recent systemd-boot boot loader and efi stub loader which carries
> >> the kernel, the cmdline, the initrd in one single EFI binary will also
> >> not use any boot loader snippets, it will be discovered by the loader
> >> itself, which parses the PE/COFF files and looks for specific content.
> >>
> >> The snippets are meant to unify the boot loader *configuration*, but
> >> they do not mean that every bootable kernel will or should have one.
> >> There might be many ways for kernels to be found by the boot loader,
> >> the snippets are just one source for that.
> >>
> >> I'm not sure what exact problem this patch tries to solve,
> >
> > rebooting with kexec is faster than a full reboot. Currently we do not
> > support kexec very well. Lennart asked for something like this, but
> perhaps
> > we no longer want to support kexec loading?
>
> I kind of miss a description what this change is supposed to support
> in the end. It can't be described with "replacing a call to a binary".
>
> Automatic searching and deciding what kernel to boot, and how to
> search for these kernels, and how all that should continue to work
> reliably in the longer run, while the boot loaders underneath improve
> and change; that is something we should define before and have a clear
> idea, before we copy only parts of that logic from the current tools.
>
> I thought you wrote a specification?
freedesktop.org/wiki/Specifications/BootLoaderSpec/

Anyways, you should at least merge this patch:

1423865887-1507-1-git-send-email-sh...@churchofgit.com

 (
https://www.mail-archive.com/systemd-devel@lists.freedesktop.org/msg28005.html
)

> Thanks,
> Kay
> ___
> systemd-devel mailing list
> systemd-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/systemd-devel
>



-- 
Shawn Landden
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

[systemd-devel] [PATCH 2/6] fix strict aliasing violations in src/udev/udev-builtin-usb_id.c

[Shawn Landden]

---
 src/udev/udev-builtin-usb_id.c | 16 
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/src/udev/udev-builtin-usb_id.c b/src/udev/udev-builtin-usb_id.c
index 6516d93..3c15b2f 100644
--- a/src/udev/udev-builtin-usb_id.c
+++ b/src/udev/udev-builtin-usb_id.c
@@ -28,6 +28,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include "udev.h"
 
@@ -179,21 +180,20 @@ static int dev_if_packed_info(struct udev_device *dev, 
char *ifs_str, size_t len
 
 ifs_str[0] = '\0';
 while (pos < size && strpos+7 < len-2) {
-struct usb_interface_descriptor *desc;
+unsigned char *desc = &buf[pos];
 char if_str[8];
 
-desc = (struct usb_interface_descriptor *) &buf[pos];
-if (desc->bLength < 3)
+if (desc[offsetof(struct usb_interface_descriptor, bLength)] < 
3)
 break;
-pos += desc->bLength;
+pos += desc[offsetof(struct usb_interface_descriptor, 
bLength)];
 
-if (desc->bDescriptorType != USB_DT_INTERFACE)
+if (desc[offsetof(struct usb_interface_descriptor, 
bDescriptorType)] != USB_DT_INTERFACE)
 continue;
 
 if (snprintf(if_str, 8, ":%02x%02x%02x",
- desc->bInterfaceClass,
- desc->bInterfaceSubClass,
- desc->bInterfaceProtocol) != 7)
+ desc[offsetof(struct usb_interface_descriptor, 
bInterfaceClass)],
+ desc[offsetof(struct usb_interface_descriptor, 
bInterfaceSubClass)],
+ desc[offsetof(struct usb_interface_descriptor, 
bInterfaceProtocol)]) != 7)
 continue;
 
 if (strstr(ifs_str, if_str) != NULL)
-- 
2.2.1.209.g41e5f3a

___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

[systemd-devel] [PATCH 6/6] refactor in_addr_to_string()

[Shawn Landden]

---
 src/resolve/resolved-dns-rr.c |  6 ++
 src/shared/in-addr-util.c | 32 +++-
 2 files changed, 13 insertions(+), 25 deletions(-)

diff --git a/src/resolve/resolved-dns-rr.c b/src/resolve/resolved-dns-rr.c
index 78d9e4a..a73ccd7 100644
--- a/src/resolve/resolved-dns-rr.c
+++ b/src/resolve/resolved-dns-rr.c
@@ -527,13 +527,11 @@ int dns_resource_record_to_string(const DnsResourceRecord 
*rr, char **ret) {
 break;
 
 case DNS_TYPE_A: {
-_cleanup_free_ char *x = NULL;
-
-r = in_addr_to_string(AF_INET, (const union in_addr_union*) 
&rr->a.in_addr, &x);
+r = in_addr_to_string(AF_INET, (const union in_addr_union*) 
&rr->a.in_addr, &t);
 if (r < 0)
 return r;
 
-s = strjoin(k, " ", x, NULL);
+s = strjoin(k, " ", t, NULL);
 if (!s)
 return -ENOMEM;
 break;
diff --git a/src/shared/in-addr-util.c b/src/shared/in-addr-util.c
index b7532a6..f23b343 100644
--- a/src/shared/in-addr-util.c
+++ b/src/shared/in-addr-util.c
@@ -22,6 +22,7 @@
 #include 
 
 #include "in-addr-util.h"
+#include "socket-util.h"
 
 int in_addr_is_null(int family, const union in_addr_union *u) {
 assert(u);
@@ -179,31 +180,20 @@ int in_addr_prefix_next(int family, union in_addr_union 
*u, unsigned prefixlen)
 }
 
 int in_addr_to_string(int family, const union in_addr_union *u, char **ret) {
-char *x;
-size_t l;
+union sockaddr_union sa;
 
-assert(u);
-assert(ret);
+sa.sa.sa_family = family;
 
-if (family == AF_INET)
-l = INET_ADDRSTRLEN;
-else if (family == AF_INET6)
-l = INET6_ADDRSTRLEN;
-else
-return -EAFNOSUPPORT;
+if (sa.sa.sa_family == AF_INET) {
+sa.in.sin_addr = u->in;
 
-x = new(char, l);
-if (!x)
-return -ENOMEM;
+return sockaddr_pretty(&sa.sa, (socklen_t)sizeof(sa.in),  
false, false, ret);
+} else if (family == AF_INET6) {
+sa.in6.sin6_addr = u->in6;
 
-errno = 0;
-if (!inet_ntop(family, u, x, l)) {
-free(x);
-return errno ? -errno : -EINVAL;
-}
-
-*ret = x;
-return 0;
+return sockaddr_pretty(&sa.sa, (socklen_t)sizeof(sa.in6), 
false, false, ret);
+} else
+return -EAFNOSUPPORT;
 }
 
 struct in6_addr in_addr_to_in6_addr(struct in_addr ipv4) {
-- 
2.2.1.209.g41e5f3a

___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

[systemd-devel] [PATCH 4/6] make in_addr_from_string() accept ipv4 addresses even when using AF_INET6

[Shawn Landden]

if we are going to have a function to fix up the deficiencies of
inet_pton(), better go all the way.
---
 src/shared/in-addr-util.c | 17 +
 src/shared/in-addr-util.h |  1 +
 2 files changed, 18 insertions(+)

diff --git a/src/shared/in-addr-util.c b/src/shared/in-addr-util.c
index ade4012..b7532a6 100644
--- a/src/shared/in-addr-util.c
+++ b/src/shared/in-addr-util.c
@@ -206,7 +206,18 @@ int in_addr_to_string(int family, const union 
in_addr_union *u, char **ret) {
 return 0;
 }
 
+struct in6_addr in_addr_to_in6_addr(struct in_addr ipv4) {
+struct in6_addr r = { { {
+0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0xFF, 0xFF, 0, 0, 0, 0
+} } };
+
+r.s6_addr32[3] = ipv4.s_addr;
+
+return r;
+}
+
 int in_addr_from_string(int family, const char *s, void *ret) {
+struct in_addr v4mapped;
 
 assert(s);
 assert(ret);
@@ -214,6 +225,12 @@ int in_addr_from_string(int family, const char *s, void 
*ret) {
 if (!IN_SET(family, AF_INET, AF_INET6))
 return -EAFNOSUPPORT;
 
+if (family == AF_INET6)
+if (inet_pton(AF_INET, s, &v4mapped) == 1) {
+*((struct in6_addr *)ret) = 
in_addr_to_in6_addr(v4mapped);
+return 0;
+}
+
 errno = 0;
 if (inet_pton(family, s, ret) <= 0)
 return errno ? -errno : -EINVAL;
diff --git a/src/shared/in-addr-util.h b/src/shared/in-addr-util.h
index 8b3a07c..f140ec9 100644
--- a/src/shared/in-addr-util.h
+++ b/src/shared/in-addr-util.h
@@ -37,6 +37,7 @@ int in_addr_equal(int family, const union in_addr_union *a, 
const union in_addr_
 int in_addr_prefix_intersect(int family, const union in_addr_union *a, 
unsigned aprefixlen, const union in_addr_union *b, unsigned bprefixlen);
 int in_addr_prefix_next(int family, union in_addr_union *u, unsigned 
prefixlen);
 int in_addr_to_string(int family, const union in_addr_union *u, char **ret);
+struct in6_addr in_addr_to_in6_addr(struct in_addr ipv4) _const_;
 int in_addr_from_string(int family, const char *s, void *ret);
 int in_addr_from_string_auto(const char *s, int *family, union in_addr_union 
*ret);
 unsigned char in_addr_netmask_to_prefixlen(const struct in_addr *addr);
-- 
2.2.1.209.g41e5f3a

___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [PATCH] fix compiler warning

[Shawn Landden]

On Sat, Mar 14, 2015 at 6:31 AM, Ronny Chevalier 
wrote:

> 2015-03-11 4:42 GMT+01:00 Shawn Landden :
> > warning: pointer/integer type mismatch in conditional expression
> > ---
> >  src/shared/socket-util.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/src/shared/socket-util.c b/src/shared/socket-util.c
> > index 5820279..73e1177 100644
> > --- a/src/shared/socket-util.c
> > +++ b/src/shared/socket-util.c
> > @@ -475,8 +475,8 @@ int sockaddr_port(const struct sockaddr *_sa) {
> >  return -EAFNOSUPPORT;
> >
> >  return ntohs(sa->sa.sa_family == AF_INET6 ?
> > -   sa->in6.sin6_port :
> > -   sa->in.sin_port);
> > +   (uint16_t)sa->in6.sin6_port :
> > +   (uint16_t)sa->in.sin_port);
> >  }
> >
>
> Hi,
>
> I don't see any warning, and the man (man netinet_in.h) says that
> sin_port and sin6_port are of type in_port_t which is equivalent to
> uint16_t.
>

in_port_t and in6_port_t. If the type returned by a ternary operator is not
identical gcc-5 warns, even though they are both backed by uint16_t and
thus there is no violation.

>
> Maybe I'm missing something?
>
> Ronny
> _______
> systemd-devel mailing list
> systemd-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/systemd-devel
>



-- 
Shawn Landden
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

[systemd-devel] [PATCH] path-lookup: use secure_getenv()

[Shawn Landden]

All these except user_data_home_dir() are certainly vectors for
arbitrary code execution. These should use secure_getenv()
---
 src/shared/path-lookup.c | 20 ++--
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/src/shared/path-lookup.c b/src/shared/path-lookup.c
index fbf46cd..0fb86df 100644
--- a/src/shared/path-lookup.c
+++ b/src/shared/path-lookup.c
@@ -33,7 +33,7 @@ int user_config_home(char **config_home) {
 const char *e;
 char *r;
 
-e = getenv("XDG_CONFIG_HOME");
+e = secure_getenv("XDG_CONFIG_HOME");
 if (e) {
 r = strappend(e, "/systemd/user");
 if (!r)
@@ -44,7 +44,7 @@ int user_config_home(char **config_home) {
 } else {
 const char *home;
 
-home = getenv("HOME");
+home = secure_getenv("HOME");
 if (home) {
 r = strappend(home, "/.config/systemd/user");
 if (!r)
@@ -62,7 +62,7 @@ int user_runtime_dir(char **runtime_dir) {
 const char *e;
 char *r;
 
-e = getenv("XDG_RUNTIME_DIR");
+e = secure_getenv("XDG_RUNTIME_DIR");
 if (e) {
 r = strappend(e, "/systemd/user");
 if (!r)
@@ -83,13 +83,13 @@ static int user_data_home_dir(char **dir, const char 
*suffix) {
  * suggests because we assume that that is a link to
  * /etc/systemd/ anyway. */
 
-e = getenv("XDG_DATA_HOME");
+e = secure_getenv("XDG_DATA_HOME");
 if (e)
 res = strappend(e, suffix);
 else {
 const char *home;
 
-home = getenv("HOME");
+home = secure_getenv("HOME");
 if (home)
 res = strjoin(home, "/.local/share", suffix, NULL);
 else
@@ -146,7 +146,7 @@ static char** user_dirs(
 if (user_runtime_dir(&runtime_dir) < 0)
 return NULL;
 
-e = getenv("XDG_CONFIG_DIRS");
+e = secure_getenv("XDG_CONFIG_DIRS");
 if (e) {
 config_dirs = strv_split(e, ":");
 if (!config_dirs)
@@ -157,7 +157,7 @@ static char** user_dirs(
 if (r < 0)
 return NULL;
 
-e = getenv("XDG_DATA_DIRS");
+e = secure_getenv("XDG_DATA_DIRS");
 if (e)
 data_dirs = strv_split(e, ":");
 else
@@ -248,7 +248,7 @@ int lookup_paths_init(
 
 /* First priority is whatever has been passed to us via env
  * vars */
-e = getenv("SYSTEMD_UNIT_PATH");
+e = secure_getenv("SYSTEMD_UNIT_PATH");
 if (e) {
 if (endswith(e, ":")) {
 e = strndupa(e, strlen(e) - 1);
@@ -340,7 +340,7 @@ int lookup_paths_init(
 #ifdef HAVE_SYSV_COMPAT
 /* /etc/init.d/ compatibility does not matter to users */
 
-e = getenv("SYSTEMD_SYSVINIT_PATH");
+e = secure_getenv("SYSTEMD_SYSVINIT_PATH");
 if (e) {
 p->sysvinit_path = path_split_and_make_absolute(e);
 if (!p->sysvinit_path)
@@ -358,7 +358,7 @@ int lookup_paths_init(
 return -ENOMEM;
 }
 
-e = getenv("SYSTEMD_SYSVRCND_PATH");
+e = secure_getenv("SYSTEMD_SYSVRCND_PATH");
 if (e) {
 p->sysvrcnd_path = path_split_and_make_absolute(e);
 if (!p->sysvrcnd_path)
-- 
2.2.1.209.g41e5f3a

___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [PATCH] fix compiler warning

[Shawn Landden]

On Sun, Mar 15, 2015 at 4:53 AM, Ronny Chevalier 
wrote:

> 2015-03-14 17:54 GMT+01:00 Shawn Landden :
> > On Sat, Mar 14, 2015 at 6:31 AM, Ronny Chevalier <
> chevalier.ro...@gmail.com>
> > wrote:
> >>
> >> 2015-03-11 4:42 GMT+01:00 Shawn Landden :
> >> > warning: pointer/integer type mismatch in conditional expression
> >> > ---
> >> >  src/shared/socket-util.c | 4 ++--
> >> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >> >
> >> > diff --git a/src/shared/socket-util.c b/src/shared/socket-util.c
> >> > index 5820279..73e1177 100644
> >> > --- a/src/shared/socket-util.c
> >> > +++ b/src/shared/socket-util.c
> >> > @@ -475,8 +475,8 @@ int sockaddr_port(const struct sockaddr *_sa) {
> >> >  return -EAFNOSUPPORT;
> >> >
> >> >  return ntohs(sa->sa.sa_family == AF_INET6 ?
> >> > -   sa->in6.sin6_port :
> >> > -   sa->in.sin_port);
> >> > +   (uint16_t)sa->in6.sin6_port :
> >> > +   (uint16_t)sa->in.sin_port);
> >> >  }
> >> >
> >>
> >> Hi,
> >>
> >> I don't see any warning, and the man (man netinet_in.h) says that
> >> sin_port and sin6_port are of type in_port_t which is equivalent to
> >> uint16_t.
> >
> >
> > in_port_t and in6_port_t. If the type returned by a ternary operator is
> not
> > identical gcc-5 warns, even though they are both backed by uint16_t and
> thus
> > there is no violation.
>
> netinet/in.h does not provide in6_port_t according to the manpage. I
> even check with the glibc and on master there is no mention of
> in6_port_t.
> Maybe you are using another libc?
>
> You are right, but has been a bug in an early gcc-5, which quite a few
incorrect warnings.

> >>
> >>
> >> Maybe I'm missing something?
> >>
> >> Ronny
> >> ___
> >> systemd-devel mailing list
> >> systemd-devel@lists.freedesktop.org
> >> http://lists.freedesktop.org/mailman/listinfo/systemd-devel
> >
> >
> >
> >
> > --
> > Shawn Landden
> ___
> systemd-devel mailing list
> systemd-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/systemd-devel
>



-- 
Shawn Landden
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [PATCH 4/6] make in_addr_from_string() accept ipv4 addresses even when using AF_INET6

[Shawn Landden]

On Sun, Mar 15, 2015 at 1:07 PM, Zbigniew Jędrzejewski-Szmek <
zbys...@in.waw.pl> wrote:

> On Wed, Mar 11, 2015 at 08:13:47AM -0700, Shawn Landden wrote:
> > if we are going to have a function to fix up the deficiencies of
> > inet_pton(), better go all the way.
> > ---
> >  src/shared/in-addr-util.c | 17 +
> >  src/shared/in-addr-util.h |  1 +
> >  2 files changed, 18 insertions(+)
> >
> > diff --git a/src/shared/in-addr-util.c b/src/shared/in-addr-util.c
> > index ade4012..b7532a6 100644
> > --- a/src/shared/in-addr-util.c
> > +++ b/src/shared/in-addr-util.c
> > @@ -206,7 +206,18 @@ int in_addr_to_string(int family, const union
> in_addr_union *u, char **ret) {
> >  return 0;
> >  }
> >
> > +struct in6_addr in_addr_to_in6_addr(struct in_addr ipv4) {
> > +struct in6_addr r = { { {
> > +0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0xFF, 0xFF, 0, 0, 0, 0
> > +} } };
> > +
> > +r.s6_addr32[3] = ipv4.s_addr;
> > +
> > +return r;
> > +}
> > +
> >  int in_addr_from_string(int family, const char *s, void *ret) {
> > +struct in_addr v4mapped;
> >
> >  assert(s);
> >  assert(ret);
> > @@ -214,6 +225,12 @@ int in_addr_from_string(int family, const char *s,
> void *ret) {
> >  if (!IN_SET(family, AF_INET, AF_INET6))
> >  return -EAFNOSUPPORT;
> >
> > +if (family == AF_INET6)
> > +if (inet_pton(AF_INET, s, &v4mapped) == 1) {
> > +*((struct in6_addr *)ret) =
> in_addr_to_in6_addr(v4mapped);
> > +return 0;
> > +}
> > +
> Hm, IIUC, an ipv6 address was specified, but the user gave an ipv4 address.
> Automatically converting seems like a way to entice confusion...
>
> Except that the linux kernel supports binding to ipv4 this way, (see
ipv6(7)).

In the inet_pton man page:

BUGS
   AF_INET6 does not recognize IPv4 addresses.  An explicit IPv4-mapped
IPv6 address must be supplied in src instead.


Zbyszek
>
> >  errno = 0;
> >  if (inet_pton(family, s, ret) <= 0)
> >  return errno ? -errno : -EINVAL;
> > diff --git a/src/shared/in-addr-util.h b/src/shared/in-addr-util.h
> > index 8b3a07c..f140ec9 100644
> > --- a/src/shared/in-addr-util.h
> > +++ b/src/shared/in-addr-util.h
> > @@ -37,6 +37,7 @@ int in_addr_equal(int family, const union
> in_addr_union *a, const union in_addr_
> >  int in_addr_prefix_intersect(int family, const union in_addr_union *a,
> unsigned aprefixlen, const union in_addr_union *b, unsigned bprefixlen);
> >  int in_addr_prefix_next(int family, union in_addr_union *u, unsigned
> prefixlen);
> >  int in_addr_to_string(int family, const union in_addr_union *u, char
> **ret);
> > +struct in6_addr in_addr_to_in6_addr(struct in_addr ipv4) _const_;
> >  int in_addr_from_string(int family, const char *s, void *ret);
> >  int in_addr_from_string_auto(const char *s, int *family, union
> in_addr_union *ret);
> >  unsigned char in_addr_netmask_to_prefixlen(const struct in_addr *addr);
> > --
> > 2.2.1.209.g41e5f3a
> >
> > ___
> > systemd-devel mailing list
> > systemd-devel@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/systemd-devel
> ___
> systemd-devel mailing list
> systemd-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/systemd-devel
>



-- 
Shawn Landden
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [PATCH 4/6] make in_addr_from_string() accept ipv4 addresses even when using AF_INET6

[Shawn Landden]

On Sun, Mar 15, 2015 at 1:36 PM, Zbigniew Jędrzejewski-Szmek <
zbys...@in.waw.pl> wrote:

> On Sun, Mar 15, 2015 at 01:28:05PM -0700, Shawn Landden wrote:
> > On Sun, Mar 15, 2015 at 1:07 PM, Zbigniew Jędrzejewski-Szmek <
> > zbys...@in.waw.pl> wrote:
> >
> > > On Wed, Mar 11, 2015 at 08:13:47AM -0700, Shawn Landden wrote:
> > > > if we are going to have a function to fix up the deficiencies of
> > > > inet_pton(), better go all the way.
> > > > ---
> > > >  src/shared/in-addr-util.c | 17 +
> > > >  src/shared/in-addr-util.h |  1 +
> > > >  2 files changed, 18 insertions(+)
> > > >
> > > > diff --git a/src/shared/in-addr-util.c b/src/shared/in-addr-util.c
> > > > index ade4012..b7532a6 100644
> > > > --- a/src/shared/in-addr-util.c
> > > > +++ b/src/shared/in-addr-util.c
> > > > @@ -206,7 +206,18 @@ int in_addr_to_string(int family, const union
> > > in_addr_union *u, char **ret) {
> > > >  return 0;
> > > >  }
> > > >
> > > > +struct in6_addr in_addr_to_in6_addr(struct in_addr ipv4) {
> > > > +struct in6_addr r = { { {
> > > > +0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0xFF, 0xFF, 0, 0, 0, 0
> > > > +} } };
> > > > +
> > > > +r.s6_addr32[3] = ipv4.s_addr;
> > > > +
> > > > +return r;
> > > > +}
> > > > +
> > > >  int in_addr_from_string(int family, const char *s, void *ret) {
> > > > +struct in_addr v4mapped;
> > > >
> > > >  assert(s);
> > > >  assert(ret);
> > > > @@ -214,6 +225,12 @@ int in_addr_from_string(int family, const char
> *s,
> > > void *ret) {
> > > >  if (!IN_SET(family, AF_INET, AF_INET6))
> > > >  return -EAFNOSUPPORT;
> > > >
> > > > +if (family == AF_INET6)
> > > > +if (inet_pton(AF_INET, s, &v4mapped) == 1) {
> > > > +*((struct in6_addr *)ret) =
> > > in_addr_to_in6_addr(v4mapped);
> > > > +return 0;
> > > > +}
> > > > +
> > > Hm, IIUC, an ipv6 address was specified, but the user gave an ipv4
> address.
> > > Automatically converting seems like a way to entice confusion...
> > >
> > > Except that the linux kernel supports binding to ipv4 this way, (see
> > ipv6(7)).
> >
> > In the inet_pton man page:
> >
> > BUGS
> >AF_INET6 does not recognize IPv4 addresses.  An explicit
> IPv4-mapped
> > IPv6 address must be supplied in src instead.
> Right, but looking at how in_addr_from_string is used, we usually call it
> with an explicit family, and if it parses as ipv4, assume the family is
> ipv4,
> otherwise parse it as ipv6. So your change would either be a noop (if
> the check for ipv4 syntax was already done before), or would mess up this
> detection.
>
> What scenario are you trying to solve?
>
> nothing is particular, I was just trying to give myself a reason not to
refactor these to just call inet_pton() directly as without this change the
wrapper function does nothing except check for NULL.

> Zbyszek
>
> >
> >
> > Zbyszek
> > >
> > > >  errno = 0;
> > > >  if (inet_pton(family, s, ret) <= 0)
> > > >  return errno ? -errno : -EINVAL;
> > > > diff --git a/src/shared/in-addr-util.h b/src/shared/in-addr-util.h
> > > > index 8b3a07c..f140ec9 100644
> > > > --- a/src/shared/in-addr-util.h
> > > > +++ b/src/shared/in-addr-util.h
> > > > @@ -37,6 +37,7 @@ int in_addr_equal(int family, const union
> > > in_addr_union *a, const union in_addr_
> > > >  int in_addr_prefix_intersect(int family, const union in_addr_union
> *a,
> > > unsigned aprefixlen, const union in_addr_union *b, unsigned
> bprefixlen);
> > > >  int in_addr_prefix_next(int family, union in_addr_union *u, unsigned
> > > prefixlen);
> > > >  int in_addr_to_string(int family, const union in_addr_union *u, char
> > > **ret);
> > > > +struct in6_addr in_addr_to_in6_addr(struct in_addr ipv4) _const_;
> > > >  int in_addr_from_string(int family, const char *s, void *ret);
> > > >  int in_addr_from_string_auto(const char *s, int *family, union
> > > in_addr_union *ret);
> > > >  unsigned char in_addr_netmask_to_prefixlen(const struct in_addr
> *addr);
> > > > --
> > > > 2.2.1.209.g41e5f3a
> > > >
> > > > ___
> > > > systemd-devel mailing list
> > > > systemd-devel@lists.freedesktop.org
> > > > http://lists.freedesktop.org/mailman/listinfo/systemd-devel
> > > ___
> > > systemd-devel mailing list
> > > systemd-devel@lists.freedesktop.org
> > > http://lists.freedesktop.org/mailman/listinfo/systemd-devel
> > >
> >
> >
> >
> > --
> > Shawn Landden
> ___
> systemd-devel mailing list
> systemd-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/systemd-devel
>



-- 
Shawn Landden
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

[systemd-devel] [PATCH] bootchart: remove duplicated code, prevent creating empty files

[Shawn Landden]

In Debian and rawhide Fedora, which have CONFIG_SCHEDSTATS=n,
bootchart creates empty files in /run/log before printing an error.
Stop doing that.

Moreover this duplicated part of the code doesn't even have error checking
so there is no error avoided by doing this early.

Reported-by: tfirg_ on IRC
Signed-off-by: Shawn Landden 
---
 src/bootchart/bootchart.c | 9 -
 1 file changed, 9 deletions(-)

diff --git a/src/bootchart/bootchart.c b/src/bootchart/bootchart.c
index 99ffb86..71dffc9 100644
--- a/src/bootchart/bootchart.c
+++ b/src/bootchart/bootchart.c
@@ -395,15 +395,6 @@ int main(int argc, char *argv[]) {
 sampledata->sampletime = gettime_ns();
 sampledata->counter = samples;
 
-if (!of && (access(arg_output_path, R_OK|W_OK|X_OK) == 0)) {
-t = time(NULL);
-r = strftime(datestr, sizeof(datestr), "%Y%m%d-%H%M", 
localtime(&t));
-assert_se(r > 0);
-
-snprintf(output_file, PATH_MAX, "%s/bootchart-%s.svg", 
arg_output_path, datestr);
-of = fopen(output_file, "we");
-}
-
 if (sysfd < 0)
 sysfd = open("/sys", O_RDONLY|O_CLOEXEC);
 
-- 
2.2.1.209.g41e5f3a

___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

[systemd-devel] [PATCH] bootchart: more useful error message for common error

[Shawn Landden]

Reported-by: tfirg_ on IRC
Signed-off-by: Shawn Landden 
---
 src/bootchart/store.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/bootchart/store.c b/src/bootchart/store.c
index 607cc5e..dfa681f 100644
--- a/src/bootchart/store.c
+++ b/src/bootchart/store.c
@@ -176,7 +176,7 @@ vmstat_next:
 /* overall CPU utilization */
 schedstat = openat(procfd, "schedstat", O_RDONLY);
 if (schedstat == -1) {
-log_error_errno(errno, "Failed to open 
/proc/schedstat: %m");
+log_error_errno(errno, "Failed to open /proc/schedstat 
(requires CONFIG_SCHEDSTATS=y in kernel config): %m");
 exit(EXIT_FAILURE);
 }
 }
-- 
2.2.1.209.g41e5f3a

___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [PATCH] timedated: Add a LocalOffset property for timezone offset

[Shawn Landden]

On Sun, Mar 22, 2015 at 10:32 PM, Lennart Poettering
 wrote:
> On Thu, 19.03.15 14:39, David Herrmann (dh.herrm...@gmail.com) wrote:
>
>> Hmm, so this is a convenience call. You could just set tm.tm_zone
>> locally and use mktime() with the value retrieved by "Timezone"? Yeah,
>> the time-api is awful with global variables, but that's not really our
>> fault, is it?
>
> This would not work, as struct tm's .tm_zone field is not sufficient
> to indicate a timezone. The code would have to set $TZ and call tzset().
>
> Given the simplicity of this I'd probably just merge Stef's
> patch... It *is* kinda nice given that the timezone database is
> constantly updated and having this exposed on the bus so that it is
> accessible remotely has the benefit that you get the actual timezone
> information in effect on the remote system, and not a possible
> out-of-date timezone from the local database. If you follow what I mean...
The patch is COMPLETELY WRONG, as timezone offsets are differn't for
differn't times due to DST. Even if this is taken into account there
are all sorts of races
with this information around DST switches.

How about a
"LocalTimeUSec" property, for those times that the application doesn't
have an interest doing timezone calculations and just wants the local
time on the other machine. In any other case I think the client has to
do proper timezone calculations.

I could write a patch for a "TZDataVersion" field that holds the tzdata release.
>
>>
>> I'm not really against this bus-call, but I also don't see the point.
>> There's much more information in a timezone file than the offset, so
>> why expose the offset but not the other data?
>>
>> Thanks
>> David
>>
>> >  static int property_get_ntp_sync(
>> >  sd_bus *bus,
>> >  const char *path,
>> > @@ -440,7 +456,8 @@ static int method_set_timezone(sd_bus *bus, 
>> > sd_bus_message *m, void *userdata, s
>> > LOG_MESSAGE("Changed time zone to '%s'.", c->zone),
>> > NULL);
>> >
>> > -sd_bus_emit_properties_changed(bus, "/org/freedesktop/timedate1", 
>> > "org.freedesktop.timedate1", "Timezone", NULL);
>> > +sd_bus_emit_properties_changed(bus, "/org/freedesktop/timedate1", 
>> > "org.freedesktop.timedate1",
>> > +   "Timezone", "LocalOffset", NULL);
>> >
>> >  return sd_bus_reply_method_return(m, NULL);
>> >  }
>> > @@ -666,6 +683,7 @@ static int method_set_ntp(sd_bus *bus, sd_bus_message 
>> > *m, void *userdata, sd_bus
>> >  static const sd_bus_vtable timedate_vtable[] = {
>> >  SD_BUS_VTABLE_START(0),
>> >  SD_BUS_PROPERTY("Timezone", "s", NULL, offsetof(Context, zone), 
>> > SD_BUS_VTABLE_PROPERTY_EMITS_CHANGE),
>> > +SD_BUS_PROPERTY("LocalOffset", "t", property_get_local_offset, 0, 
>> > SD_BUS_VTABLE_PROPERTY_EMITS_CHANGE),
>> >  SD_BUS_PROPERTY("LocalRTC", "b", bus_property_get_bool, 
>> > offsetof(Context, local_rtc), SD_BUS_VTABLE_PROPERTY_EMITS_CHANGE),
>> >  SD_BUS_PROPERTY("CanNTP", "b", bus_property_get_bool, 
>> > offsetof(Context, can_ntp), 0),
>> >  SD_BUS_PROPERTY("NTP", "b", bus_property_get_bool, 
>> > offsetof(Context, use_ntp), SD_BUS_VTABLE_PROPERTY_EMITS_CHANGE),
>> > --
>> > 2.3.3
>> >
>> > ___
>> > systemd-devel mailing list
>> > systemd-devel@lists.freedesktop.org
>> > http://lists.freedesktop.org/mailman/listinfo/systemd-devel
>> ___
>> systemd-devel mailing list
>> systemd-devel@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/systemd-devel
>
>
> Lennart
>
> --
> Lennart Poettering, Red Hat
> ___
> systemd-devel mailing list
> systemd-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/systemd-devel



-- 
Liberty equality fraternity or death,

Shawn Landden
ChurchOfGit.com
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

[systemd-devel] [PATCH] timedated: add LocalTimeUSec via dbus

[Shawn Landden]

---
 src/timedate/timedated.c | 24 
 1 file changed, 24 insertions(+)

diff --git a/src/timedate/timedated.c b/src/timedate/timedated.c
index ca771d5..f83b99c 100644
--- a/src/timedate/timedated.c
+++ b/src/timedate/timedated.c
@@ -38,6 +38,7 @@
 #include "bus-common-errors.h"
 #include "event-util.h"
 #include "selinux-util.h"
+#include "time-util.h"
 
 #define NULL_ADJTIME_UTC "0.0 0 0\n0\nUTC\n"
 #define NULL_ADJTIME_LOCAL "0.0 0 0\n0\nLOCAL\n"
@@ -361,6 +362,28 @@ static int property_get_time(
 return sd_bus_message_append(reply, "t", now(CLOCK_REALTIME));
 }
 
+/* gmtime(LocalTimeUSec % USEC_PER_SEC) is useful as long as you ignore GNU
+ * extensions of tm_gmtoff and tm_zone, and their ourgrowth in strftime(3)
+ */
+static int property_get_local_time(
+sd_bus *bus,
+const char *path,
+const char *interface,
+const char *property,
+sd_bus_message *reply,
+void *userdata,
+sd_bus_error *error)
+{
+struct tm tm;
+struct timespec ts;
+usec_t t;
+
+t = now(CLOCK_REALTIME);
+(void)timespec_store(&ts, t);
+assert_se(localtime_r(&dummy, &ts.tv_sec));
+return sd_bus_message_append(reply, "t", t + (tm->tm_gmtoff * 
USEC_PER_SEC));
+}
+
 static int property_get_ntp_sync(
 sd_bus *bus,
 const char *path,
@@ -671,6 +694,7 @@ static const sd_bus_vtable timedate_vtable[] = {
 SD_BUS_PROPERTY("NTP", "b", bus_property_get_bool, offsetof(Context, 
use_ntp), SD_BUS_VTABLE_PROPERTY_EMITS_CHANGE),
 SD_BUS_PROPERTY("NTPSynchronized", "b", property_get_ntp_sync, 0, 0),
 SD_BUS_PROPERTY("TimeUSec", "t", property_get_time, 0, 0),
+SD_BUS_PROPERTY("LocalTimeUSec", "t", property_get_local_time, 0, 0),
 SD_BUS_PROPERTY("RTCTimeUSec", "t", property_get_rtc_time, 0, 0),
 SD_BUS_METHOD("SetTime", "xbb", NULL, method_set_time, 
SD_BUS_VTABLE_UNPRIVILEGED),
 SD_BUS_METHOD("SetTimezone", "sb", NULL, method_set_timezone, 
SD_BUS_VTABLE_UNPRIVILEGED),
-- 
2.2.1.209.g41e5f3a

___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

[systemd-devel] [PATCH] timedatectl: fix when queried system has differn't timezone

[Shawn Landden]

Also allow getting time from time(2) when BUS_TRANSPORT_MACHINE.
---
 src/timedate/timedatectl.c | 7 +--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/src/timedate/timedatectl.c b/src/timedate/timedatectl.c
index 9e04f8f..7c9bd11 100644
--- a/src/timedate/timedatectl.c
+++ b/src/timedate/timedatectl.c
@@ -106,14 +106,17 @@ static void print_status_info(const StatusInfo *i) {
 
 /* Enforce the values of /etc/localtime */
 if (getenv("TZ")) {
-fprintf(stderr, "Warning: Ignoring the TZ variable. Reading 
the system's time zone setting only.\n\n");
+fprintf(stderr, "Warning: Ignoring the TZ variable.\n\n");
 unsetenv("TZ");
 }
 
+setenv("TZ", i->timezone, false);
+tzset();
+
 if (i->time != 0) {
 sec = (time_t) (i->time / USEC_PER_SEC);
 have_time = true;
-} else if (arg_transport == BUS_TRANSPORT_LOCAL) {
+} else if (IN_SET(arg_transport, BUS_TRANSPORT_REMOTE, 
BUS_TRANSPORT_MACHINE)) {
 sec = time(NULL);
 have_time = true;
 } else
-- 
2.2.1.209.g41e5f3a

___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [PATCH] timedated: Add a LocalOffset property for timezone offset

[Shawn Landden]

On Mon, Mar 23, 2015 at 4:42 AM, Stef Walter  wrote:
> On 23.03.2015 12:11, Shawn Landden wrote:
>> On Sun, Mar 22, 2015 at 10:32 PM, Lennart Poettering
>>  wrote:
>>> On Thu, 19.03.15 14:39, David Herrmann (dh.herrm...@gmail.com) wrote:
>>>
>>>> Hmm, so this is a convenience call. You could just set tm.tm_zone
>>>> locally and use mktime() with the value retrieved by "Timezone"? Yeah,
>>>> the time-api is awful with global variables, but that's not really our
>>>> fault, is it?
>>>
>>> This would not work, as struct tm's .tm_zone field is not sufficient
>>> to indicate a timezone. The code would have to set $TZ and call tzset().
>>>
>>> Given the simplicity of this I'd probably just merge Stef's
>>> patch... It *is* kinda nice given that the timezone database is
>>> constantly updated and having this exposed on the bus so that it is
>>> accessible remotely has the benefit that you get the actual timezone
>>> information in effect on the remote system, and not a possible
>>> out-of-date timezone from the local database. If you follow what I mean...
>> The patch is COMPLETELY WRONG, as timezone offsets are differn't for
>> differn't times due to DST. Even if this is taken into account there
>> are all sorts of races
>> with this information around DST switches.
>
> As with the TimeUSec and RTCTimeUSec properties, it's assumed the caller
> knows the information is out of date the instant they receive it.
Your patch is a whole hour slow during daylight savings time. (The
epoch, when you query the offset for, is standard time as it is during
January.)
>
> But in that case I guess we shouldn't do
> SD_BUS_VTABLE_PROPERTY_EMITS_CHANGE...
Fixed.
>
>> How about a
>> "LocalTimeUSec" property, for those times that the application doesn't
>> have an interest doing timezone calculations and just wants the local
>> time on the other machine.
>
> That's fine with me. Although IMO it's quite unorthodox to have local
> time in an integer value counted since the epoch.
>
> 
>
> Stef
>



-- 
Liberty equality fraternity or death,

Shawn Landden
ChurchOfGit.com
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

[systemd-devel] [PATCH] timedatectl: fix when queried system has differn't timezone

[Shawn Landden]

Also allow getting time from time(2) when BUS_TRANSPORT_MACHINE.

v2: check for error
---
 src/timedate/timedatectl.c | 11 +--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/src/timedate/timedatectl.c b/src/timedate/timedatectl.c
index 9e04f8f..44d329e 100644
--- a/src/timedate/timedatectl.c
+++ b/src/timedate/timedatectl.c
@@ -106,14 +106,21 @@ static void print_status_info(const StatusInfo *i) {
 
 /* Enforce the values of /etc/localtime */
 if (getenv("TZ")) {
-fprintf(stderr, "Warning: Ignoring the TZ variable. Reading 
the system's time zone setting only.\n\n");
+fprintf(stderr, "Warning: Ignoring the TZ variable.\n\n");
 unsetenv("TZ");
 }
 
+r = setenv("TZ", i->timezone, false);
+if (r < 0) {
+log_error_errno(errno, "Failed to set TZ environment variable: 
%m");
+exit(EXIT_FAILURE);
+}
+tzset();
+
 if (i->time != 0) {
 sec = (time_t) (i->time / USEC_PER_SEC);
 have_time = true;
-} else if (arg_transport == BUS_TRANSPORT_LOCAL) {
+} else if (IN_SET(arg_transport, BUS_TRANSPORT_REMOTE, 
BUS_TRANSPORT_MACHINE)) {
 sec = time(NULL);
 have_time = true;
 } else
-- 
2.2.1.209.g41e5f3a

___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [PATCH] timedated: Add a LocalOffset property for timezone offset

[Shawn Landden]

On Mon, Mar 23, 2015 at 8:56 AM, Kay Sievers  wrote:
> On Mon, Mar 23, 2015 at 3:49 PM, Stef Walter  wrote:
>> On 23.03.2015 15:26, Kay Sievers wrote:
>>> On Mon, Mar 23, 2015 at 1:46 PM, David Herrmann  
>>> wrote:
>>>> Hi
>>>>
>>>> On Mon, Mar 23, 2015 at 6:32 AM, Lennart Poettering
>>>>  wrote:
>>>>> On Thu, 19.03.15 14:39, David Herrmann (dh.herrm...@gmail.com) wrote:
>>>>>
>>>>>> Hmm, so this is a convenience call. You could just set tm.tm_zone
>>>>>> locally and use mktime() with the value retrieved by "Timezone"? Yeah,
>>>>>> the time-api is awful with global variables, but that's not really our
>>>>>> fault, is it?
>>>>>
>>>>> This would not work, as struct tm's .tm_zone field is not sufficient
>>>>> to indicate a timezone. The code would have to set $TZ and call tzset().
>>>>
>>>> I see. The Unix-way of doing things!
>>>>
>>>>> Given the simplicity of this I'd probably just merge Stef's
>>>>> patch... It *is* kinda nice given that the timezone database is
>>>>> constantly updated and having this exposed on the bus so that it is
>>>>> accessible remotely has the benefit that you get the actual timezone
>>>>> information in effect on the remote system, and not a possible
>>>>> out-of-date timezone from the local database. If you follow what I mean...
>>>>
>>>> ..locale data is also updated regularly but we don't export
>>>> locale-files on the bus ;)
>>>>
>>>> Anyway, if we add pseudo-redundant APIs, I'd go with a "LocalTime"
>>>> field as Shawn suggested. This is what the bus-user is interested in,
>>>> right? If they need more data (like DST), they should just parse
>>>> zoneinfo. And with "LocalTime" we avoid any ambiguity regarding DST.
>>>
>>> Transmitting several absolute time stamps sounds wrong.
>>> Transmitting the offset sounds as wrong as tz_minuteswest is and it got 
>>> killed
>>> for that reason.
>>>
>>> Why does anybody ever need anything else than the actual UTC time and
>>> the configured  timezone of the machine?
>>
>> Yes, we do. In Cockpit we want to show the server's local time, in the
>> server's timezone. This is similar to how GNOME shows you the local time
>> in the local timezone.
>>
>> But we don't have easy access to libc and its zoneinfo file parser from
>> javascript running in a browser. We do have access to DBus [0]
>
> But it is not systemd's task to cover for missing functionality in the
> cockpit architecture. We should not add redundant interfaces just
> provide data for a specific user.
>
> The data systemd provides over the bus is the the same as the running
> system provides to the local user: the UTC time + the time zone.
> Everything else should be a task of the presentation, in this case
> cockpit.
>
>> So obviously we could invent a DBus service called timedated2 (or
>> whatever) to do what we need ... but I figured I'd see if timedated
>> could do what we needed first.
>
> I don't think so. Systemd covers operating system data, but I doubt it
> should provide "remote glibc" functionality.
>
>> It seems that timedatectl itself needs information about remote local
>> time, since when connecting remotely over DBus it gets the local time
>> wrong. [1]
>
> Yeah, it's currently a mix of local vs. remote data. If timedatctl
> should work correctly on a remote host, it needs to fixed.
I posted a patch for this.
>
> Kay
> ___
> systemd-devel mailing list
> systemd-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/systemd-devel



-- 
Liberty equality fraternity or death,

Shawn Landden
ChurchOfGit.com
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [PATCH] timedated: Add a LocalOffset property for timezone offset

[Shawn Landden]

On Mon, Mar 23, 2015 at 12:13 PM, Stef Walter  wrote:
> Sorry about the encrypted email ... I hit the wrong button.
>
> On 23.03.2015 19:07, Shawn Landden wrote:
>> On Mon, Mar 23, 2015 at 8:56 AM, Kay Sievers  wrote:
>>> On Mon, Mar 23, 2015 at 3:49 PM, Stef Walter  wrote:
>>>> On 23.03.2015 15:26, Kay Sievers wrote:
>>>>> On Mon, Mar 23, 2015 at 1:46 PM, David Herrmann  
>>>>> wrote:
>>>>>> Hi
>>>>>>
>>>>>> On Mon, Mar 23, 2015 at 6:32 AM, Lennart Poettering
>>>>>>  wrote:
>>>>>>> On Thu, 19.03.15 14:39, David Herrmann (dh.herrm...@gmail.com) wrote:
>>>>>>>
>>>>>>>> Hmm, so this is a convenience call. You could just set tm.tm_zone
>>>>>>>> locally and use mktime() with the value retrieved by "Timezone"? Yeah,
>>>>>>>> the time-api is awful with global variables, but that's not really our
>>>>>>>> fault, is it?
>>>>>>>
>>>>>>> This would not work, as struct tm's .tm_zone field is not sufficient
>>>>>>> to indicate a timezone. The code would have to set $TZ and call tzset().
>>>>>>
>>>>>> I see. The Unix-way of doing things!
>>>>>>
>>>>>>> Given the simplicity of this I'd probably just merge Stef's
>>>>>>> patch... It *is* kinda nice given that the timezone database is
>>>>>>> constantly updated and having this exposed on the bus so that it is
>>>>>>> accessible remotely has the benefit that you get the actual timezone
>>>>>>> information in effect on the remote system, and not a possible
>>>>>>> out-of-date timezone from the local database. If you follow what I 
>>>>>>> mean...
>>>>>>
>>>>>> ..locale data is also updated regularly but we don't export
>>>>>> locale-files on the bus ;)
>>>>>>
>>>>>> Anyway, if we add pseudo-redundant APIs, I'd go with a "LocalTime"
>>>>>> field as Shawn suggested. This is what the bus-user is interested in,
>>>>>> right? If they need more data (like DST), they should just parse
>>>>>> zoneinfo. And with "LocalTime" we avoid any ambiguity regarding DST.
>>>>>
>>>>> Transmitting several absolute time stamps sounds wrong.
>>>>> Transmitting the offset sounds as wrong as tz_minuteswest is and it got 
>>>>> killed
>>>>> for that reason.
>>>>>
>>>>> Why does anybody ever need anything else than the actual UTC time and
>>>>> the configured  timezone of the machine?
>>>>
>>>> Yes, we do. In Cockpit we want to show the server's local time, in the
>>>> server's timezone. This is similar to how GNOME shows you the local time
>>>> in the local timezone.
>>>>
>>>> But we don't have easy access to libc and its zoneinfo file parser from
>>>> javascript running in a browser. We do have access to DBus [0]
>>>
>>> But it is not systemd's task to cover for missing functionality in the
>>> cockpit architecture. We should not add redundant interfaces just
>>> provide data for a specific user.
>
> Even systemd can't reliably consume its own DBus timedated interface
> remotely. Even once timedatectl is fixed, the information will be wrong
> unless the version of zoneinfo and glibc are identical on both systems
> involved.
>
> This is hardly Cockpit specific.
>
>>> The data systemd provides over the bus is the the same as the running
>>> system provides to the local user: the UTC time + the time zone.
>>> Everything else should be a task of the presentation, in this case
>>> cockpit.
>
> The timezone timedated provides is in presentation format (ie:
> "Australia/Tasmania"), a non-standard identifier, rather than a actual
> information about the timezone. Unless all systems agree on these names
> and have identical versions of the data (ie: zoneinfo), identical
> algorithms for interpreting it (ie: glibc) it is impossible to correctly
> interpret it remotely.
all that would be required would be a hash of the zoneinfo file
>
> There are several DBus interfaces that possible not remotable, whether
> intentionally or not.
>
> For example NetwortkManager has byte[4] arrays stored in uint32's that
> are completely unusable on a different architecture ... other DBus
> interfaces have local file names as properties ... as so on.
>
> We should mark the timedated interface as non-remotable or call out the
> remotability problems in the documentation.
The problem really isn't this bad. With my patch to actually use the
remove timezone setting things are relatively OK. as is documented in
tzset() glibc falls back to UTC.
>
>>> Yeah, it's currently a mix of local vs. remote data. If timedatctl
>>> should work correctly on a remote host, it needs to fixed.
>> I posted a patch for this.
>
> With the current DBus interface, the only way to reliably fix this is to
> exec timedatectl remotely on the other system and pipe the output data.
> See above.
>
> Stef
>



-- 
Liberty equality fraternity or death,

Shawn Landden
ChurchOfGit.com
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [PATCH] timedated: add LocalTimeUSec via dbus

[Shawn Landden]

On Mon, Mar 23, 2015 at 6:52 PM, Zbigniew Jędrzejewski-Szmek
 wrote:
> On Mon, Mar 23, 2015 at 04:24:38AM -0700, Shawn Landden wrote:
>> ---
>>  src/timedate/timedated.c | 24 
>>  1 file changed, 24 insertions(+)
>>
>> diff --git a/src/timedate/timedated.c b/src/timedate/timedated.c
>> index ca771d5..f83b99c 100644
>> --- a/src/timedate/timedated.c
>> +++ b/src/timedate/timedated.c
>> @@ -38,6 +38,7 @@
>>  #include "bus-common-errors.h"
>>  #include "event-util.h"
>>  #include "selinux-util.h"
>> +#include "time-util.h"
>>
>>  #define NULL_ADJTIME_UTC "0.0 0 0\n0\nUTC\n"
>>  #define NULL_ADJTIME_LOCAL "0.0 0 0\n0\nLOCAL\n"
>> @@ -361,6 +362,28 @@ static int property_get_time(
>>  return sd_bus_message_append(reply, "t", now(CLOCK_REALTIME));
>>  }
>>
>> +/* gmtime(LocalTimeUSec % USEC_PER_SEC) is useful as long as you ignore GNU
>> + * extensions of tm_gmtoff and tm_zone, and their ourgrowth in strftime(3)
>> + */
>> +static int property_get_local_time(
>> +sd_bus *bus,
>> +const char *path,
>> +const char *interface,
>> +const char *property,
>> +sd_bus_message *reply,
>> +void *userdata,
>> +sd_bus_error *error)
>> +{
>> +struct tm tm;
>> +struct timespec ts;
>> +usec_t t;
>> +
>> +t = now(CLOCK_REALTIME);
>> +(void)timespec_store(&ts, t);
>> +assert_se(localtime_r(&dummy, &ts.tv_sec));
>> +return sd_bus_message_append(reply, "t", t + (tm->tm_gmtoff * 
>> USEC_PER_SEC));
>> +}
>> +
>>  static int property_get_ntp_sync(
>>  sd_bus *bus,
>>  const char *path,
>> @@ -671,6 +694,7 @@ static const sd_bus_vtable timedate_vtable[] = {
>>  SD_BUS_PROPERTY("NTP", "b", bus_property_get_bool, 
>> offsetof(Context, use_ntp), SD_BUS_VTABLE_PROPERTY_EMITS_CHANGE),
>>  SD_BUS_PROPERTY("NTPSynchronized", "b", property_get_ntp_sync, 0, 
>> 0),
>>  SD_BUS_PROPERTY("TimeUSec", "t", property_get_time, 0, 0),
>> +SD_BUS_PROPERTY("LocalTimeUSec", "t", property_get_local_time, 0, 
>> 0),
>>  SD_BUS_PROPERTY("RTCTimeUSec", "t", property_get_rtc_time, 0, 0),
>>  SD_BUS_METHOD("SetTime", "xbb", NULL, method_set_time, 
>> SD_BUS_VTABLE_UNPRIVILEGED),
>>  SD_BUS_METHOD("SetTimezone", "sb", NULL, method_set_timezone, 
>> SD_BUS_VTABLE_UNPRIVILEGED)
>
> This patch seems the most reasonable of the proposed solutions.
> It answers the question "What does the remote host think local time is?"
> unambigously.
>
> The other patch proposed by Shawn mixes the remote UTC time with local
> information about timezones, which doesn't seem as clean.
These are not for the same problem. We need both. I want to ship a
checksum of the zoneinfo file with the timezone, but its hard to do
this cleanly as using coreutils from C is a PITA.
>
> Kay said:
>> Transmitting several absolute time stamps sounds wrong.
> The second timestamp is what the user sees, so in some way it is a
> very significant thing. It is the outcome of the remote knowledge
> about the time and its config and timezone database. We aren't making
> any promises about the accuracy of that, but just trying to accurately
> convey that information. Seems fine to me to transmit this bit of
> information.
>
> Zbyszek
> ___
> systemd-devel mailing list
> systemd-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/systemd-devel



-- 
Shawn Landden
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

[systemd-devel] [PATCH 1/2] timedatectl: check for getenv("TZDIR")

[Shawn Landden]

I liked having the DST information. It is a pity glibc doesn't export
this information.
---
 src/timedate/timedatectl.c | 29 +++--
 1 file changed, 19 insertions(+), 10 deletions(-)

diff --git a/src/timedate/timedatectl.c b/src/timedate/timedatectl.c
index ab5c8a1..8daae54 100644
--- a/src/timedate/timedatectl.c
+++ b/src/timedate/timedatectl.c
@@ -74,7 +74,7 @@ typedef struct StatusInfo {
 
 static void print_status_info(const StatusInfo *i) {
 char a[FORMAT_TIMESTAMP_MAX];
-struct tm tm;
+struct tm localtime, gmtime;
 time_t sec;
 bool have_time = false;
 _cleanup_free_ char *zc = NULL, *zn = NULL;
@@ -84,13 +84,18 @@ static void print_status_info(const StatusInfo *i) {
 
 /* Enforce the values of /etc/localtime */
 if (getenv("TZ")) {
-fprintf(stderr, "Warning: Ignoring the TZ variable.\n\n");
+fprintf(stderr, "Warning: Ignoring the %s variable.\n\n", 
"TZ");
 unsetenv("TZ");
 }
 
+if (getenv("TZDIR")) {
+fprintf(stderr, "Warning: Ignoring the %s variable.\n\n", 
"TZDIR");
+unsetenv("TZDIR");
+}
+
 r = setenv("TZ", i->timezone, false);
 if (r < 0) {
-log_error_errno(errno, "Failed to set TZ environment variable: 
%m");
+log_error_errno(errno, "Failed to set %s environment variable: 
%m", "TZ");
 exit(EXIT_FAILURE);
 }
 tzset();
@@ -105,27 +110,30 @@ static void print_status_info(const StatusInfo *i) {
 fprintf(stderr, "Warning: Could not get time from timedated 
and not operating locally.\n\n");
 
 if (have_time) {
-xstrftime(a, "%a %Y-%m-%d %H:%M:%S %Z", localtime_r(&sec, 
&tm));
+localtime_r(&sec, &localtime);
+gmtime_r(&sec, &gmtime);
+
+xstrftime(a, "%a %Y-%m-%d %H:%M:%S %Z", localtime);
 printf("  Local time: %.*s\n", (int) sizeof(a), a);
 
-xstrftime(a, "%a %Y-%m-%d %H:%M:%S UTC", gmtime_r(&sec, &tm));
+xstrftime(a, "%a %Y-%m-%d %H:%M:%S UTC", gmtime);
 printf("  Universal time: %.*s\n", (int) sizeof(a), a);
 } else {
-printf("  Local time: %s\n", "n/a");
-printf("  Universal time: %s\n", "n/a");
+printf("  Local time: %.*s\n", (int) strlen("n/a"), "n/a");
+printf("  Universal time: %.*s\n", (int) strlen("n/a"), "n/a");
 }
 
 if (i->rtc_time > 0) {
 time_t rtc_sec;
 
 rtc_sec = (time_t)(i->rtc_time / USEC_PER_SEC);
-xstrftime(a, "%a %Y-%m-%d %H:%M:%S", gmtime_r(&rtc_sec, &tm));
+xstrftime(a, "%a %Y-%m-%d %H:%M:%S", gmtime);
 printf("RTC time: %.*s\n", (int) sizeof(a), a);
 } else
 printf("RTC time: %s\n", "n/a");
 
 if (have_time)
-xstrftime(a, "%Z, %z", localtime_r(&sec, &tm));
+xstrftime(a, "%Z, %z", localtime);
 
 printf("   Time zone: %s (%.*s)\n"
" NTP enabled: %s\n"
@@ -142,7 +150,8 @@ static void print_status_info(const StatusInfo *i) {
   " mode can not be fully supported. It will 
create various problems with time\n"
   " zone changes and daylight saving time 
adjustments. The RTC time is never updated,\n"
   " it relies on external facilities to maintain 
it. If at all possible, use\n"
-  " RTC in UTC by calling 'timedatectl 
set-local-rtc 0'" ANSI_HIGHLIGHT_OFF ".\n", stdout);
+  " RTC in UTC by calling 'timedatectl 
set-local-rtc 0'\n
+  " For more details see 
http://www.cl.cam.ac.uk/~mgk25/mswish/ut-rtc.html"; ANSI_HIGHLIGHT_OFF ".\n", 
stdout);
 }
 
 static int show_status(sd_bus *bus, char **args, unsigned n) {
-- 
2.2.1.209.g41e5f3a

___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

[systemd-devel] [PATCH 2/2] macro: allow assert_se() assertions to also be optimized out when NDEBUG is set

[Shawn Landden]

Will result in slightly smaller binaries, and cuts out the branch, even if
the expression is still executed.
---
 src/shared/macro.h | 12 ++--
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/src/shared/macro.h b/src/shared/macro.h
index 7f89951..02219ea 100644
--- a/src/shared/macro.h
+++ b/src/shared/macro.h
@@ -212,17 +212,17 @@ static inline unsigned long ALIGN_POWER2(unsigned long u) 
{
 (__x / __y + !!(__x % __y));\
 })
 
-#define assert_se(expr) \
-do {\
-if (_unlikely_(!(expr)))\
-log_assert_failed(#expr, __FILE__, __LINE__, 
__PRETTY_FUNCTION__); \
-} while (false) \
-
 /* We override the glibc assert() here. */
 #undef assert
 #ifdef NDEBUG
+#define assert_se(expr) do {expr} while(false)
 #define assert(expr) do {} while(false)
 #else
+#define assert_se(expr) \
+do {\
+if (_unlikely_(!(expr)))\
+log_assert_failed(#expr, __FILE__, __LINE__, 
__PRETTY_FUNCTION__); \
+} while (false)
 #define assert(expr) assert_se(expr)
 #endif
 
-- 
2.2.1.209.g41e5f3a

___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

[systemd-devel] [PATCH] CODING_STYLE: this also help with unaligned memory accesses

[Shawn Landden]

And those arches don't get much testing too.
---
 CODING_STYLE | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/CODING_STYLE b/CODING_STYLE
index b687e72..8934954 100644
--- a/CODING_STYLE
+++ b/CODING_STYLE
@@ -14,7 +14,8 @@
 - The destructors always unregister the object from the next bigger
   object, not the other way around
 
-- To minimize strict aliasing violations, we prefer unions over casting
+- To minimize strict aliasing violations and unaligned memory accesses,
+  we prefer unions over casting
 
 - For robustness reasons, destructors should be able to destruct
   half-initialized objects, too
-- 
2.2.1.209.g41e5f3a

___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [PATCH 1/2] timedatectl: check for getenv("TZDIR")

[Shawn Landden]

On Tue, Mar 24, 2015 at 11:32 AM, Kay Sievers  wrote:
> On Tue, Mar 24, 2015 at 7:11 PM, Shawn Landden  wrote:
>
>>  /* Enforce the values of /etc/localtime */
>>  if (getenv("TZ")) {
>> -fprintf(stderr, "Warning: Ignoring the TZ variable.\n\n");
>> +fprintf(stderr, "Warning: Ignoring the %s variable.\n\n", 
>> "TZ");
>
> What style is that? A second static string inserted with %s?
> Wanto de-duplicate the first static string? :)
yep
>
>> -xstrftime(a, "%a %Y-%m-%d %H:%M:%S UTC", gmtime_r(&sec, 
>> &tm));
>> +xstrftime(a, "%a %Y-%m-%d %H:%M:%S UTC", gmtime);
>
> Why this change?
localtime_r() and gmtime_r() are called multiple times.
>
>> +  " RTC in UTC by calling 'timedatectl 
>> set-local-rtc 0'\n
>> +  " For more details see 
>> http://www.cl.cam.ac.uk/~mgk25/mswish/ut-rtc.html"; ANSI_HIGHLIGHT_OFF ".\n", 
>> stdout);
>
> Hmm, I find links to "random" web pages in error output a bit too
> unconventional.
OK, but the page has been around for years and does tell Windows users
what registry entry to set to use UTC for their RTC.
>
> Kay
> ___
> systemd-devel mailing list
> systemd-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/systemd-devel



-- 
Shawn Landden
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

[systemd-devel] [PATCH] timedatectl: check for getenv("TZDIR")

[Shawn Landden]

I liked having the DST information. It is a pity glibc doesn't export
this information.

avoid calling gmtime_r() and localtime_r() twice
deduplicate some strings

v2
---
 src/timedate/timedatectl.c | 24 
 1 file changed, 16 insertions(+), 8 deletions(-)

diff --git a/src/timedate/timedatectl.c b/src/timedate/timedatectl.c
index ab5c8a1..8073111 100644
--- a/src/timedate/timedatectl.c
+++ b/src/timedate/timedatectl.c
@@ -74,7 +74,7 @@ typedef struct StatusInfo {
 
 static void print_status_info(const StatusInfo *i) {
 char a[FORMAT_TIMESTAMP_MAX];
-struct tm tm;
+struct tm localtime, gmtime;
 time_t sec;
 bool have_time = false;
 _cleanup_free_ char *zc = NULL, *zn = NULL;
@@ -84,10 +84,15 @@ static void print_status_info(const StatusInfo *i) {
 
 /* Enforce the values of /etc/localtime */
 if (getenv("TZ")) {
-fprintf(stderr, "Warning: Ignoring the TZ variable.\n\n");
+fprintf(stderr, "Warning: Ignoring the %s variable.\n\n", 
"TZ");
 unsetenv("TZ");
 }
 
+if (getenv("TZDIR")) {
+fprintf(stderr, "Warning: Ignoring the %s variable.\n\n", 
"TZDIR");
+unsetenv("TZDIR");
+}
+
 r = setenv("TZ", i->timezone, false);
 if (r < 0) {
 log_error_errno(errno, "Failed to set TZ environment variable: 
%m");
@@ -105,27 +110,30 @@ static void print_status_info(const StatusInfo *i) {
 fprintf(stderr, "Warning: Could not get time from timedated 
and not operating locally.\n\n");
 
 if (have_time) {
-xstrftime(a, "%a %Y-%m-%d %H:%M:%S %Z", localtime_r(&sec, 
&tm));
+localtime_r(&sec, &localtime);
+gmtime_r(&sec, &gmtime);
+
+xstrftime(a, "%a %Y-%m-%d %H:%M:%S %Z", localtime);
 printf("  Local time: %.*s\n", (int) sizeof(a), a);
 
-xstrftime(a, "%a %Y-%m-%d %H:%M:%S UTC", gmtime_r(&sec, &tm));
+xstrftime(a, "%a %Y-%m-%d %H:%M:%S UTC", gmtime);
 printf("  Universal time: %.*s\n", (int) sizeof(a), a);
 } else {
-printf("  Local time: %s\n", "n/a");
-printf("  Universal time: %s\n", "n/a");
+printf("  Local time: %.*s\n", (int) strlen("n/a"), "n/a");
+printf("  Universal time: %.*s\n", (int) strlen("n/a"), "n/a");
 }
 
 if (i->rtc_time > 0) {
 time_t rtc_sec;
 
 rtc_sec = (time_t)(i->rtc_time / USEC_PER_SEC);
-xstrftime(a, "%a %Y-%m-%d %H:%M:%S", gmtime_r(&rtc_sec, &tm));
+xstrftime(a, "%a %Y-%m-%d %H:%M:%S", gmtime);
 printf("RTC time: %.*s\n", (int) sizeof(a), a);
 } else
 printf("RTC time: %s\n", "n/a");
 
 if (have_time)
-xstrftime(a, "%Z, %z", localtime_r(&sec, &tm));
+xstrftime(a, "%Z, %z", localtime);
 
 printf("   Time zone: %s (%.*s)\n"
" NTP enabled: %s\n"
-- 
2.2.1.209.g41e5f3a

___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

[systemd-devel] [PATCH] timedatectl: check for getenv("TZDIR")

[Shawn Landden]

I liked having the DST information. It is a pity glibc doesn't export
this information.

v3
---
 src/timedate/timedatectl.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/src/timedate/timedatectl.c b/src/timedate/timedatectl.c
index ab5c8a1..d529a0a 100644
--- a/src/timedate/timedatectl.c
+++ b/src/timedate/timedatectl.c
@@ -88,6 +88,11 @@ static void print_status_info(const StatusInfo *i) {
 unsetenv("TZ");
 }
 
+if (getenv("TZDIR")) {
+fprintf(stderr, "Warning: Ignoring the TZDIR variable.\n\n");
+unsetenv("TZDIR");
+}
+
 r = setenv("TZ", i->timezone, false);
 if (r < 0) {
 log_error_errno(errno, "Failed to set TZ environment variable: 
%m");
-- 
2.2.1.209.g41e5f3a

___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [PATCH 2/2] macro: allow assert_se() assertions to also be optimized out when NDEBUG is set

[Shawn Landden]

On Thu, Mar 26, 2015 at 1:19 AM, Lennart Poettering
 wrote:
> On Tue, 24.03.15 11:11, Shawn Landden (sh...@churchofgit.com) wrote:
>
>> Will result in slightly smaller binaries, and cuts out the branch, even if
>> the expression is still executed.
>
> I am sorry, but the whole point of assert_se() is that it has side
> effects. That's why it is called that way (the "se" suffix stands for
> "side effects"), and is different from assert(). You are supposed to
> use it whenever the code enclosed in it shall not be suppressed if
> NDEBUG is defined. This patch of yours breaks that whole logic!

+#define assert_se(expr) do {expr} while(false)
 #define assert(expr) do {} while(false)

No it does not. see "{expr}", it still is doing the side effect. It
just doesn't check if the return value is as expected.
>
> Lennart
>
>> ---
>>  src/shared/macro.h | 12 ++--
>>  1 file changed, 6 insertions(+), 6 deletions(-)
>>
>> diff --git a/src/shared/macro.h b/src/shared/macro.h
>> index 7f89951..02219ea 100644
>> --- a/src/shared/macro.h
>> +++ b/src/shared/macro.h
>> @@ -212,17 +212,17 @@ static inline unsigned long ALIGN_POWER2(unsigned long 
>> u) {
>>  (__x / __y + !!(__x % __y));\
>>  })
>>
>> -#define assert_se(expr) \
>> -do {\
>> -if (_unlikely_(!(expr)))\
>> -log_assert_failed(#expr, __FILE__, __LINE__, 
>> __PRETTY_FUNCTION__); \
>> -} while (false) \
>> -
>>  /* We override the glibc assert() here. */
>>  #undef assert
>>  #ifdef NDEBUG
>> +#define assert_se(expr) do {expr} while(false)
>>  #define assert(expr) do {} while(false)
>>  #else
>> +#define assert_se(expr) \
>> +do {\
>> +if (_unlikely_(!(expr)))\
>> +log_assert_failed(#expr, __FILE__, __LINE__, 
>> __PRETTY_FUNCTION__); \
>> +} while (false)
>>  #define assert(expr) assert_se(expr)
>>  #endif
>>
>> --
>> 2.2.1.209.g41e5f3a
>>
>> ___
>> systemd-devel mailing list
>> systemd-devel@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/systemd-devel
>
>
> Lennart
>
> --
> Lennart Poettering, Red Hat
> ___
> systemd-devel mailing list
> systemd-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/systemd-devel



-- 
Shawn Landden
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [PATCH 2/2] macro: allow assert_se() assertions to also be optimized out when NDEBUG is set

[Shawn Landden]

On Thu, Mar 26, 2015 at 5:47 PM, Djalal Harouni  wrote:
> On Fri, Mar 27, 2015 at 12:30:53AM +0100, Tom Gundersen wrote:
>> On Thu, Mar 26, 2015 at 9:19 AM, Lennart Poettering
>>  wrote:
>> > On Tue, 24.03.15 11:11, Shawn Landden (sh...@churchofgit.com) wrote:
>> >
>> >> Will result in slightly smaller binaries, and cuts out the branch, even if
>> >> the expression is still executed.
>> >
>> > I am sorry, but the whole point of assert_se() is that it has side
>> > effects. That's why it is called that way (the "se" suffix stands for
>> > "side effects"), and is different from assert(). You are supposed to
>> > use it whenever the code enclosed in it shall not be suppressed if
>> > NDEBUG is defined. This patch of yours breaks that whole logic!
>>
>> Hm, am I reading the patch wrong? It looks good to me. It is simply
>> dropping the branching and logging in the NDEBUG case, but the
>> expression itself is still evaluated.
> Yep but it seems that abort() will never be called,
> log_assert_failed() => abort()
>
> And the logging mechanism is also one of those side effects. IMO unless
> there are real valid reasons for any change to these macors... changing
> anything here will just bring more bugs that we may never notice.
>
Those are the side effects of assert(), the side effects of the
expression are still evaluated.
>
>> So in the NDEBUG case "assert_se(foo())" becomes equivalent to
>> "foo()", which I guess makes sense (though I doubt it makes much of a
>> difference).
>>
>> -t
>>
>> >> ---
>> >>  src/shared/macro.h | 12 ++--
>> >>  1 file changed, 6 insertions(+), 6 deletions(-)
>> >>
>> >> diff --git a/src/shared/macro.h b/src/shared/macro.h
>> >> index 7f89951..02219ea 100644
>> >> --- a/src/shared/macro.h
>> >> +++ b/src/shared/macro.h
>> >> @@ -212,17 +212,17 @@ static inline unsigned long ALIGN_POWER2(unsigned 
>> >> long u) {
>> >>  (__x / __y + !!(__x % __y));\
>> >>  })
>> >>
>> >> -#define assert_se(expr) \
>> >> -do {\
>> >> -if (_unlikely_(!(expr)))\
>> >> -log_assert_failed(#expr, __FILE__, __LINE__, 
>> >> __PRETTY_FUNCTION__); \
>> >> -} while (false) \
>> >> -
>> >>  /* We override the glibc assert() here. */
>> >>  #undef assert
>> >>  #ifdef NDEBUG
>> >> +#define assert_se(expr) do {expr} while(false)
>> >>  #define assert(expr) do {} while(false)
>> >>  #else
>> >> +#define assert_se(expr) \
>> >> +do {\
>> >> +if (_unlikely_(!(expr)))\
>> >> +log_assert_failed(#expr, __FILE__, __LINE__, 
>> >> __PRETTY_FUNCTION__); \
>> >> +} while (false)
>> >>  #define assert(expr) assert_se(expr)
>> >>  #endif
>> >>
>> >> --
>> >> 2.2.1.209.g41e5f3a
>> >>
>> >> ___
>> >> systemd-devel mailing list
>> >> systemd-devel@lists.freedesktop.org
>> >> http://lists.freedesktop.org/mailman/listinfo/systemd-devel
>> >
>> >
>> > Lennart
>> >
>> > --
>> > Lennart Poettering, Red Hat
>> > ___
>> > systemd-devel mailing list
>> > systemd-devel@lists.freedesktop.org
>> > http://lists.freedesktop.org/mailman/listinfo/systemd-devel
>> ___
>> systemd-devel mailing list
>> systemd-devel@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/systemd-devel
>
> --
> Djalal Harouni
> http://opendz.org
> ___
> systemd-devel mailing list
> systemd-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/systemd-devel



-- 
Shawn Landden
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [PATCH] CODING_STYLE: this also help with unaligned memory accesses

[Shawn Landden]

On Thu, Mar 26, 2015 at 1:31 AM, Lennart Poettering
 wrote:
> On Tue, 24.03.15 11:16, Shawn Landden (sh...@churchofgit.com) wrote:
>
>> And those arches don't get much testing too.
>> ---
>>  CODING_STYLE | 3 ++-
>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/CODING_STYLE b/CODING_STYLE
>> index b687e72..8934954 100644
>> --- a/CODING_STYLE
>> +++ b/CODING_STYLE
>> @@ -14,7 +14,8 @@
>>  - The destructors always unregister the object from the next bigger
>>object, not the other way around
>>
>> -- To minimize strict aliasing violations, we prefer unions over casting
>> +- To minimize strict aliasing violations and unaligned memory accesses,
>> +  we prefer unions over casting
>
> Unaligned memory accesses are an orthogonal problem really. I don't
> see how the change above really makes sense?
casting often leads to unaligned memory accesses. Without casting the
compiler usually gets it right, but yeah this has little to do with
unions. I was confusing with having a packed and unpacked struct and
converting between the two like thus:

typedef struct {
uint8_t a
uint32_t b
} packed __attribute__((packed));

#if HAVE_FAST_UNALIGNED
#define unpacked packed
#elsif
typedef struct {
uint8_t a
// uint8_t __padding[3]; gcc will do this for you
uint32_t b
} unpacked;
#endif

foo get_foo(p *void) {
  unpacked e;
  e.a = (*packed)->a;
  e.b = (*packed)->b;
  return e;
}
>
> Lennart
>
> --
> Lennart Poettering, Red Hat
> _______
> systemd-devel mailing list
> systemd-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/systemd-devel



-- 
Shawn Landden
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

[systemd-devel] [PATCH] timedatectl: check for getenv("TZDIR")

[Shawn Landden]

I liked having the DST information. It is a pity glibc doesn't export
this information.

If TZDIR is set, glibc will look there rather than /usr/share/zoneinfo.
See tzset(3).
---
 src/timedate/timedatectl.c | 8 +++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/src/timedate/timedatectl.c b/src/timedate/timedatectl.c
index 1d10c19..b10ab82 100644
--- a/src/timedate/timedatectl.c
+++ b/src/timedate/timedatectl.c
@@ -81,12 +81,18 @@ static void print_status_info(const StatusInfo *i) {
 
 assert(i);
 
-/* Enforce the values of /etc/localtime */
+/* Enforce the values of /etc/localtime, or remote /etc/localtime*/
 if (getenv("TZ")) {
 fprintf(stderr, "Warning: Ignoring the TZ variable.\n\n");
 unsetenv("TZ");
 }
 
+/* also use /usr/share/zoneinfo */
+if (getenv("TZDIR")) {
+fprintf(stderr, "Warning: Ignoring the TZDIR variable. Using 
system zoneinfo data.\n\n");
+unsetenv("TZDIR");
+}
+
 r = setenv("TZ", i->timezone, false);
 if (r < 0) {
 log_error_errno(errno, "Failed to set TZ environment variable: 
%m");
-- 
2.2.1.209.g41e5f3a

___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [PATCH 2/2] macro: allow assert_se() assertions to also be optimized out when NDEBUG is set

[Shawn Landden]

On Fri, Mar 27, 2015 at 8:16 AM, Tom Gundersen  wrote:
> On Fri, Mar 27, 2015 at 2:04 PM, Djalal Harouni  wrote:
>> Hi Shawn,
>>
>> On Thu, Mar 26, 2015 at 11:21:54PM -0700, Shawn Landden wrote:
>>> On Thu, Mar 26, 2015 at 5:47 PM, Djalal Harouni  wrote:
>>> > On Fri, Mar 27, 2015 at 12:30:53AM +0100, Tom Gundersen wrote:
>>> >> On Thu, Mar 26, 2015 at 9:19 AM, Lennart Poettering
>>> >>  wrote:
>>> >> > On Tue, 24.03.15 11:11, Shawn Landden (sh...@churchofgit.com) wrote:
>>> >> >
>>> >> >> Will result in slightly smaller binaries, and cuts out the branch, 
>>> >> >> even if
>>> >> >> the expression is still executed.
>>> >> >
>>> >> > I am sorry, but the whole point of assert_se() is that it has side
>>> >> > effects. That's why it is called that way (the "se" suffix stands for
>>> >> > "side effects"), and is different from assert(). You are supposed to
>>> >> > use it whenever the code enclosed in it shall not be suppressed if
>>> >> > NDEBUG is defined. This patch of yours breaks that whole logic!
>>> >>
>>> >> Hm, am I reading the patch wrong? It looks good to me. It is simply
>>> >> dropping the branching and logging in the NDEBUG case, but the
>>> >> expression itself is still evaluated.
>>> > Yep but it seems that abort() will never be called,
>>> > log_assert_failed() => abort()
>>> >
>>> > And the logging mechanism is also one of those side effects. IMO unless
>>> > there are real valid reasons for any change to these macors... changing
>>> > anything here will just bring more bugs that we may never notice.
>>> >
>>> Those are the side effects of assert(), the side effects of the
>>> expression are still evaluated.
>> Yes but there are also the following points:
>> * assert() is simple, assert_se() is more complex and it may modify other
>> global sate. I don't think that we can break down side effects of
>> assert_se() into:
>>  1) side effects of assert()
>>  2) side effects of other expressions.
>>
>> And then remove that part 1)
>>
>>
>> So given the fact that asser_se() do not depend on NDEBUG, did you
>> consider the following:
>>
>> * You don't have control over what the expression do, it may just call
>>   abort() or exit() in case of fatal errors, so even if you remove the
>>   explicit abort() call, expressions may just call it.
>
> If expr() aborts or exits, then it really does not matter whether we
> abort() explicitly or not, so I don't see what this has to do with
> this patch. Sure, even with NDEBUG the program may abort if that is
> called explicitly...
>
>> * Some expressions were perhaps depending on the logging mechanism which
>>   is part of the side effect. Callers to assert_se() are perhaps using
>>   it for a reason, are you sure that you are not introducing a
>>   regression here ?! it will be difficult to debug the error if we don't
>>   have logs.
>
> As far as I can tell all the relevant logging functions are pure. They
> should not interact with global state, but maybe I'm missing
> something?
>
>> * Current expression may modify/interact with a global state which may
>>   cause a fatal error, and if the caller wants to know if that failed,
>>   then abort(), your patch just introduced a regression, without the
>>   explicit abort(), all callers now have to call abort().
>
> Yeah, this is the point I think. I still think the patch makes sense
> though, it really don't see why there should be a distinction between
> assert_se() and assert() when it comes to logging and aborting.
>
> If assert_se() fails it indicates we may have messed up the global
> state, and if assert() fails it indicates that the global state may be
> messed up (by someone else). Either way the global state is
> potentially messed up and not logging/aborting seems as (un)safe in
> both situations.
>
> Another way of seeing it, intuitively I don't see why we should
> distinguish between
>
> assert_se(foo() >= 0);
>
> and
>
> r = foo();
> assert(r >= 0);
Yes. The use case is that embedded people don't want all the strings
that the logging adds to their binaries when these are ASSERTS, they
are only expected to catch programming errors. It is not to make it
faster, I think that is negligible.
>
>> * Did you grep for assert_se() uses in the source ?
>>
>> I really do not think this is an improvement, we can't predict the
>> semantics of expression here, perhaps we can with simple assert() but
>> not with assert_se().
>>
>> --
>> Djalal Harouni
>> http://opendz.org
> ___
> systemd-devel mailing list
> systemd-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/systemd-devel



-- 
Shawn Landden
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] systemd not honoring LD_LIBRARY_PATH?

[Shawn Landden]

On Fri, Mar 27, 2015 at 1:56 PM, Smith, Kenneth <
ksmit...@jaguarlandrover.com> wrote:

> I am trying to start a service that requires using a library that has the
> same name as a preexisting one (it is a patched version). It is located in
> a different path so that if I want to start it at the command line I can do
> this:
>
> LD_LIBRARY_PATH=/opt/foo/lib /usr/bin/foo_c
>
> But when I do this:
> Environment=LD_LIBRARY_PATH=/opt/foo/lib
> ExecStart=/usr/bin/foo_c
>
> ldd shows foo_c is using the wrong library.
>
> Is there a way to make this work?
>
> --
> Kind Regards,
>
> *Kenneth F. Smith*
> Linux C++ Engineer
>
> *T:* 971.256.9740  |  *M:* 503-880-6256
> *Email:* ksmit...@jaguarlandrover.com
>
> Jaguar Land Rover North America, LLC
> Open Source Technology Center
> 1419 Northwest 14th Avenue, Portland, OR 97209
> JaguarUSA.com <http://www.jaguarusa.com/index.html>  |  LandRoverUSA.com
> <http://www.landrover.com/us/en/lr/>
>
> Email: ksmit...@jaguarlandrover.com 
> PGP: RSA 2048/2048 979C6B958B89909D
> ---
> Business Details:
> Jaguar Land Rover Limited
> Registered Office: Abbey Road, Whitley, Coventry CV3 4LF
> Registered in England No: 1672070
>
> This e-mail and any attachments contain confidential information for a
> specific individual and purpose.  The information is private and privileged
> and intended solely for the use of the individual to whom it is addressed.
> If you are not the intended recipient, please e-mail us immediately.  We
> apologise for any inconvenience caused but you are hereby notified that any
> disclosure, copying or distribution or the taking of any action in reliance
> on the information contained herein is strictly prohibited.
>
You should be careful that this disclaimer doesn't come with any code you
submit, or we can't look at it.

>
> This e-mail does not constitute an order for goods or services unless
> accompanied by an official purchase order.
>
> ___
> systemd-devel mailing list
> systemd-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/systemd-devel
>
>


-- 
Liberty equality fraternity or death,

Shawn Landden
ChurchOfGit.com
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] Drop systemd-ui

[Shawn Landden]

On Mon, Mar 30, 2015 at 1:35 PM, "Jóhann B. Guðmundsson"
 wrote:
> Heyja
>
> Should this not be dropped and *DE write,integrate/implement an graphical
> frontend to systemd for themselves?
>
> It's not like this is receiving the love it needs, hence I'm pretty sure
> nobody is using this.
Parts of systemd arn't getting the love they need either, such as
systemctl show.
>
> JBG
> ___
> systemd-devel mailing list
> systemd-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/systemd-devel



-- 
Liberty equality fraternity or death,

Shawn Landden
ChurchOfGit.com
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] Drop systemd-ui

[Shawn Landden]

On Mon, Mar 30, 2015 at 4:02 PM, "Jóhann B. Guðmundsson"
 wrote:
>
>
> On 03/30/2015 10:32 PM, Shawn Landden wrote:
>>
>> On Mon, Mar 30, 2015 at 1:35 PM, "Jóhann B. Guðmundsson"
>>  wrote:
>>>
>>> Heyja
>>>
>>> Should this not be dropped and *DE write,integrate/implement an graphical
>>> frontend to systemd for themselves?
>>>
>>> It's not like this is receiving the love it needs, hence I'm pretty sure
>>> nobody is using this.
>>
>> Parts of systemd arn't getting the love they need either, such as
>> systemctl show.
>
>
> For systemd ui the *DE communities are better suited to implement, integrate
> and maintain an ui on top of systemd for their *DE.
https://wiki.gnome.org/Apps/Logs

I see the dbus type problems you are probably getting. Isn't this
actually a bug in systemd as we are still exporting as systemd1? If we
are incompatible shouldn't we be systemd2?
>
> What do you feel is missing from systemctl show?
It is only suppose to show fields that have been changed by humans
(even the developer) not systemd defaults.
>
> JBG



-- 
Liberty equality fraternity or death,

Shawn Landden
ChurchOfGit.com
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [PATCH 2/2] macro: allow assert_se() assertions to also be optimized out when NDEBUG is set

[Shawn Landden]

On Mon, Mar 30, 2015 at 5:04 PM, Djalal Harouni  wrote:
> On Fri, Mar 27, 2015 at 09:51:26AM -0700, Shawn Landden wrote:
>> On Fri, Mar 27, 2015 at 8:16 AM, Tom Gundersen  wrote:
> [...]
>> >> * Current expression may modify/interact with a global state which may
>> >>   cause a fatal error, and if the caller wants to know if that failed,
>> >>   then abort(), your patch just introduced a regression, without the
>> >>   explicit abort(), all callers now have to call abort().
>> >
>> > Yeah, this is the point I think. I still think the patch makes sense
>> > though, it really don't see why there should be a distinction between
>> > assert_se() and assert() when it comes to logging and aborting.
>> >
>> > If assert_se() fails it indicates we may have messed up the global
>> > state, and if assert() fails it indicates that the global state may be
>> > messed up (by someone else). Either way the global state is
>> > potentially messed up and not logging/aborting seems as (un)safe in
>> > both situations.
>> >
>> > Another way of seeing it, intuitively I don't see why we should
>> > distinguish between
>> >
>> > assert_se(foo() >= 0);
>> >
>> > and
>> >
>> > r = foo();
>> > assert(r >= 0);
>> Yes. The use case is that embedded people don't want all the strings
>> that the logging adds to their binaries when these are ASSERTS, they
>> are only expected to catch programming errors. It is not to make it
>> faster, I think that is negligible.
> Hmm embedded cases are real, I had to deal with some in the past. But
> not sure here since I didn't see any numbers before/after stripping, but
> perhaps you can start by updating the callers and their semantics if you
> think that the code there is robust and it's worth it... ?
How about we leave the abort() and just drop the logging with NDEBUG set?
I think the performance impact is negligible, I just want to drop all
the strings.
Perhaps we could replace the logging with backtrace() in this case?
>
> Thanks!
>
> --
> Djalal Harouni
> http://opendz.org
> ___
> systemd-devel mailing list
> systemd-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/systemd-devel



-- 
Shawn Landden
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

[systemd-devel] [PATCH] macro: allow assert_se() assertions to also be optimized when NDEBUG is set

[Shawn Landden]

replaces log with assert() to remove strings.

saves 3kB from text section of systemd.
---
 src/shared/macro.h | 16 ++--
 1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/src/shared/macro.h b/src/shared/macro.h
index 7f89951..8cbff01 100644
--- a/src/shared/macro.h
+++ b/src/shared/macro.h
@@ -212,17 +212,21 @@ static inline unsigned long ALIGN_POWER2(unsigned long u) 
{
 (__x / __y + !!(__x % __y));\
 })
 
-#define assert_se(expr) \
-do {\
-if (_unlikely_(!(expr)))\
-log_assert_failed(#expr, __FILE__, __LINE__, 
__PRETTY_FUNCTION__); \
-} while (false) \
-
 /* We override the glibc assert() here. */
 #undef assert
 #ifdef NDEBUG
+#define assert_se(expr) \
+do {\
+if (_unlikely_(!(expr)))\
+abort();\
+} while (false)
 #define assert(expr) do {} while(false)
 #else
+#define assert_se(expr) \
+do {\
+if (_unlikely_(!(expr)))\
+log_assert_failed(#expr, __FILE__, __LINE__, 
__PRETTY_FUNCTION__); \
+} while (false)
 #define assert(expr) assert_se(expr)
 #endif
 
-- 
2.2.1.209.g41e5f3a

___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel