Re: [systemd-devel] accelerometer mount-matrix quirks

[Thomas H.P. Andersen]

On Tue, Nov 22, 2016 at 5:26 PM, Bastien Nocera  wrote:

> On Tue, 2016-11-22 at 17:23 +0100, Lennart Poettering wrote:
> > On Tue, 22.11.16 12:13, Bastien Nocera (had...@hadess.net) wrote:
> >
> > > Hey,
> > >
> > > I'm adding support for reading the mount-matrix[1] from
> > > accelerometer
> > > devices in iio-sensor-proxy, but we'll need to add a way to
> > > override
> > > the mount-matrix in case the data from the device-tree is
> > > incorrect, or
> > > missing[2].
> > >
> > > I was wondering whether we should ship the hwdb quirks in systemd
> > > or
> > > iio-sensor-proxy.
> > >
> > > Opinions?
> >
> > If it's generic enough then the hwdb is definitely a good place for
> > things like that. If the concept however only has value for
> > iio-sensor-proxy specifically then it should probably live in that
> > package. Of course the lines are blurry there, so compare with the
> > input case: hwdb carries dpi quirk data for all kinds of mice, and if
> > this data is like that, then it fits in.
>
> The quirks apply to the devices, and are in the same format as what's
> offered by the kernel when the firmware provides it, so it's not iio-
> sensor-proxy specific in any way. If we were to replace iio-sensor-
> proxy, we'd still use this metadata.
>
> I'll send a patch in as soon as I've done testing on a machine that
> does need this quirking.
>

Hi Bastien,

I recently opened a PR to iio-sensor-proxy with a quirk for my laptop.
Since this is my main computer testing is easy. Let me know if I can help.
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/systemd-devel

[systemd-devel] test-selinux fails under the address sanitizer

[Thomas H.P. Andersen]

Hi,

I was wondering if anybody else had looked into this already. When building
with --enable-address-sanitizer I get the following error in test-selinux.
While trying the create a reduced test case I found that the problem comes
and goes when I remove logging either before or after the call where if
fails in src/teset/test-selinux.c. As far as I can tell the input to
log_internalv looks correct. So I suspect this could be a bug in libasan
perhaps? Did any one else ever look at this?

$ ./test-selinux
 test_testing ==
mac_selinux_use → no
mac_selinux_have → no
mac_selinux_use → no
mac_selinux_have → no
 test_loading ==
mac_selinux_init → 0 (No such file or directory) 0.00s
 test_misc ==
ASAN:DEADLYSIGNAL
=
==19627==ERROR: AddressSanitizer: SEGV on unknown address 0x41b58ab3
(pc 0x7fc021669f12 bp 0x7ffdd8e50810 sp 0x7ffdd8e4ff38 T0)
#0 0x7fc021669f11  (/lib64/libasan.so.3+0xdef11)
#1 0x7fc021619c5f  (/lib64/libasan.so.3+0x8ec5f)
#2 0x7fc02161bd7d in __interceptor_vsnprintf
(/lib64/libasan.so.3+0x90d7d)
#3 0x558369c50106 in log_internalv src/basic/log.c:677
#4 0x558369c50303 in log_internal src/basic/log.c:694
#5 0x558369c47113 in test_misc src/test/test-selinux.c:81
#6 0x558369c47574 in main src/test/test-selinux.c:117
#7 0x7fc020d9a720 in __libc_start_main (/lib64/libc.so.6+0x20720)
#8 0x558369c467c8 in _start (/home/phomes/systemd/test-selinux+0x267c8)

AddressSanitizer can not provide additional info.
SUMMARY: AddressSanitizer: SEGV (/lib64/libasan.so.3+0xdef11)
==19627==ABORTING
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [ANNOUNCE] systemd-225 around the corner

[Thomas H.P. Andersen]

On Wed, Aug 26, 2015 at 12:40 AM, David Herrmann dh.herrm...@gmail.com wrote:
 Hi

 Trying to continue with our bi-weekly release schedule, we plan to
 release version 225 tomorrow. Please give it a spin and make sure
 there is no major breakage before the release.

Hi,

There is a mem leak triggered when running
src/libsystemd-network/test-dhcp6-client.c. It would be good to know
if that is from the test code or from dhcp6. I will probably not have
time to check it myself before the release though.

 Right now, the changes consist of mostly bugfixes and resolved
 reworks. I'll commit the full NEWS tomorrow.

 Thanks
 David
 ___
 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

Re: [systemd-devel] [PATCH 4/5] import/pull: Tag replaced with reference

[Thomas H.P. Andersen]

On Thu, May 7, 2015 at 5:47 PM, Pavel Odvody podv...@redhat.com wrote:
 Signed-off-by: Pavel Odvody podv...@redhat.com
 ---
  src/import/pull.c | 28 +---
  1 file changed, 17 insertions(+), 11 deletions(-)

 diff --git a/src/import/pull.c b/src/import/pull.c
 index ef7b035..8054612 100644
 --- a/src/import/pull.c
 +++ b/src/import/pull.c
 @@ -227,7 +227,7 @@ static void on_dkr_finished(DkrPull *pull, int error, 
 void *userdata) {
  static int pull_dkr(int argc, char *argv[], void *userdata) {
  _cleanup_(dkr_pull_unrefp) DkrPull *pull = NULL;
  _cleanup_event_unref_ sd_event *event = NULL;
 -const char *name, *tag, *local;
 +const char *name, *reference, *local, *digest;
  int r;

  if (!arg_dkr_index_url) {
 @@ -240,13 +240,19 @@ static int pull_dkr(int argc, char *argv[], void 
 *userdata) {
  return -EINVAL;
  }

 -tag = strchr(argv[1], ':');
 -if (tag) {
 -name = strndupa(argv[1], tag - argv[1]);
 -tag++;
 +digest = strchr(argv[1], '@');
 +if (digest) {
 +reference = digest + 1;
 +name = strndupa(argv[1], digest - argv[1]);
 +}
 +
 +reference = strchr(argv[1], ':');
 +if (reference) {
 +name = strndupa(argv[1], reference - argv[1]);
 +reference++;
  } else {
  name = argv[1];
 -tag = latest;
 +reference = latest;
  }

This part does not look correct. Any value that we set for
reference/name in the digest part will be overwritten in the next
block. I do not know the format of the string so I cannot create a
patch for this. Can you take a look Pavel?


  if (!dkr_name_is_valid(name)) {
 @@ -254,8 +260,8 @@ static int pull_dkr(int argc, char *argv[], void 
 *userdata) {
  return -EINVAL;
  }

 -if (!dkr_tag_is_valid(tag)) {
 -log_error(Tag name '%s' is not valid., tag);
 +if (!dkr_ref_is_valid(reference)) {
 +log_error(Tag name '%s' is not valid., reference);
  return -EINVAL;
  }

 @@ -288,9 +294,9 @@ static int pull_dkr(int argc, char *argv[], void 
 *userdata) {
  }
  }

 -log_info(Pulling '%s' with tag '%s', saving as '%s'., 
 name, tag, local);
 +log_info(Pulling '%s' with reference '%s', saving as 
 '%s'., name, reference, local);
  } else
 -log_info(Pulling '%s' with tag '%s'., name, tag);
 +log_info(Pulling '%s' with reference '%s'., name, 
 reference);

  r = sd_event_default(event);
  if (r  0)
 @@ -304,7 +310,7 @@ static int pull_dkr(int argc, char *argv[], void 
 *userdata) {
  if (r  0)
  return log_error_errno(r, Failed to allocate puller: %m);

 -r = dkr_pull_start(pull, name, tag, local, arg_force);
 +r = dkr_pull_start(pull, name, reference, local, arg_force, PULL_V2);
  if (r  0)
  return log_error_errno(r, Failed to pull image: %m);

 --
 2.1.0




 ___
 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

Re: [systemd-devel] [PATCH] hostname: Allow comments in /etc/hostname

[Thomas H.P. Andersen]

On Mon, May 18, 2015 at 11:33 AM, Martin Pitt martin.p...@ubuntu.com wrote:
 Hello all,

 We got a report [1] that systemd doesn't allow comments in
 /etc/hostname. If there is a comment, your host name ends up being
 # comment. But the original hostname(1) tool documents that comments
 are allowed, thus in the sysvinit/upstart world these worked fine.
 Earlier versions of cloud-init also produced a comment there.

 This currently uses read_full_file() and strv_split_newlines() which
 is relatively expensive (although, in most cases there will only be
 one line). In cases like these should we rather use an fgets() loop?

I think FOREACH_LINE would make this more readable.

 Given that host names are limited to 64 bytes (HOST_NAME_MAX) this
 could be written more efficiently.
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] 60-persistent-storage.rules: add NVMe disks and partitions (again)

[Thomas H.P. Andersen]

On Fri, May 15, 2015 at 7:30 PM, Lennart Poettering
lenn...@poettering.net wrote:
 On Fri, 15.05.15 18:41, Per Bergqvist (p...@bst.lu) wrote:

 OK, now in git-format-patch.

 There's no patch in this mails of yours...

I see patches (nvme_id.patch) attached to Per's last two emails.
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [PATCH] fsck: define fsck return codes

[Thomas H.P. Andersen]

On Tue, May 12, 2015 at 6:17 PM, Lennart Poettering
lenn...@poettering.net wrote:
 On Tue, 12.05.15 01:00, Lennart Poettering (lenn...@poettering.net) wrote:

 On Tue, 12.05.15 00:58, Thomas H.P. Andersen (pho...@gmail.com) wrote:
 
  +#define FSCK_NO_ERROR 0
  +#define FSCK_ERROR_CORRECTED 1
  +#define FSCK_SYSTEM_SHOULD_REBOOT 2
  +#define FSCK_ERRORS_LEFT_UNCORRECTED 4
  +#define FSCK_OPERATIONAL_ERROR 8
  +#define FSCK_USAGE_OR_SYNTAX_ERROR 16
  +#define FSCK_USER_CANCELLED 32
  +#define FSCK_SHARED_LIB_ERROR 128

 Makes sense, but can you make this an enum? Also, please add a comment
 referencing the documentation in fsck(8) for this.

 I mean an anonymous enum btw. I.e. just:

 enum {
  FSCK_SUCCESS = 0,
  FSCK_ERROR_CORRECTED = 1,
  FSCK_SYSTEM_SHOULD_REBOOT = 2,
  ...
 };

Thanks. I pushed the patch with your suggestions.
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

[systemd-devel] [PATCH] fsck: define fsck return codes

[Thomas H.P. Andersen]

From: Thomas Hindoe Paaboel Andersen pho...@gmail.com

---
 src/fsck/fsck.c | 17 +
 1 file changed, 13 insertions(+), 4 deletions(-)

diff --git a/src/fsck/fsck.c b/src/fsck/fsck.c
index 56d880a..5177adc 100644
--- a/src/fsck/fsck.c
+++ b/src/fsck/fsck.c
@@ -42,6 +42,15 @@
 #include path-util.h
 #include socket-util.h
 
+#define FSCK_NO_ERROR 0
+#define FSCK_ERROR_CORRECTED 1
+#define FSCK_SYSTEM_SHOULD_REBOOT 2
+#define FSCK_ERRORS_LEFT_UNCORRECTED 4
+#define FSCK_OPERATIONAL_ERROR 8
+#define FSCK_USAGE_OR_SYNTAX_ERROR 16
+#define FSCK_USER_CANCELLED 32
+#define FSCK_SHARED_LIB_ERROR 128
+
 static bool arg_skip = false;
 static bool arg_force = false;
 static bool arg_show_progress = false;
@@ -424,7 +433,7 @@ int main(int argc, char *argv[]) {
 cmdline[i++] = NULL;
 
 execv(cmdline[0], (char**) cmdline);
-_exit(8); /* Operational error */
+_exit(FSCK_OPERATIONAL_ERROR);
 }
 
 progress_pipe[1] = safe_close(progress_pipe[1]);
@@ -448,10 +457,10 @@ int main(int argc, char *argv[]) {
 
 r = -EINVAL;
 
-if (status.si_code == CLD_EXITED  (status.si_status  2)  
root_directory)
+if (status.si_code == CLD_EXITED  (status.si_status  
FSCK_SYSTEM_SHOULD_REBOOT)  root_directory)
 /* System should be rebooted. */
 start_target(SPECIAL_REBOOT_TARGET);
-else if (status.si_code == CLD_EXITED  (status.si_status  
6))
+else if (status.si_code == CLD_EXITED  (status.si_status  
(FSCK_SYSTEM_SHOULD_REBOOT | FSCK_ERRORS_LEFT_UNCORRECTED)))
 /* Some other problem */
 start_target(SPECIAL_EMERGENCY_TARGET);
 else {
@@ -462,7 +471,7 @@ int main(int argc, char *argv[]) {
 } else
 r = 0;
 
-if (status.si_code == CLD_EXITED  (status.si_status  1))
+if (status.si_code == CLD_EXITED  (status.si_status  
FSCK_ERROR_CORRECTED))
 (void) touch(/run/systemd/quotacheck);
 
 finish:
-- 
2.4.0

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

Re: [systemd-devel] [PATCH 2/2] sysv-generator: remove NULL pointer dereference

[Thomas H.P. Andersen]

Hi Shawn,

I fixed this a few hours ago. I also updated the status in coverity.
Is there something else I can do to avoid duplicated work?

On Sun, Apr 26, 2015 at 7:58 PM, Shawn Landden sh...@churchofgit.com wrote:
 (coverity)
 ---
  src/sysv-generator/sysv-generator.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

 diff --git a/src/sysv-generator/sysv-generator.c 
 b/src/sysv-generator/sysv-generator.c
 index 5ecd750..714ce8f 100644
 --- a/src/sysv-generator/sysv-generator.c
 +++ b/src/sysv-generator/sysv-generator.c
 @@ -922,7 +922,7 @@ finish:
  int main(int argc, char *argv[]) {
  int r, q;
  _cleanup_lookup_paths_free_ LookupPaths lp = {};
 -_cleanup_(free_sysvstub_hashmapp) Hashmap *all_services;
 +_cleanup_(free_sysvstub_hashmapp) Hashmap *all_services = NULL;
  SysvStub *service;
  Iterator j;

 --
 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

Re: [systemd-devel] [PATCH 1/2] path-util: fix fd_is_mount_point

[Thomas H.P. Andersen]

On Sun, Apr 26, 2015 at 7:58 PM, Shawn Landden sh...@churchofgit.com wrote:
 (coverity)
 ---
  src/shared/path-util.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

 diff --git a/src/shared/path-util.c b/src/shared/path-util.c
 index 925bb28..95bfafc 100644
 --- a/src/shared/path-util.c
 +++ b/src/shared/path-util.c
 @@ -627,7 +627,7 @@ fallback_fstat:
  a.st_ino == b.st_ino)
  return 1;

 -return check_st_dev  (a.st_dev != b.st_dev);
 +return (check_st_dev  (a.st_dev != b.st_dev));
  }

I committed a different fix for this a few hours ago. I initialize
check_st_dev to true so we do the st_dev comparison if mnt_id is not
supported by the kernel.
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [PATCH 2/2] sysv-generator: remove NULL pointer dereference

[Thomas H.P. Andersen]

On Sun, Apr 26, 2015 at 8:23 PM, Shawn Landden sh...@churchofgit.com wrote:
 Actually you missed that free_sysvstub_hashmap does not tolerate NULL 
 pointers.
Indeed. I will commit that.
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [PATCH 2/2] sysv-generator: remove NULL pointer dereference

[Thomas H.P. Andersen]

On Sun, Apr 26, 2015 at 8:31 PM, Thomas H.P. Andersen pho...@gmail.com wrote:
 On Sun, Apr 26, 2015 at 8:23 PM, Shawn Landden sh...@churchofgit.com wrote:
 Actually you missed that free_sysvstub_hashmap does not tolerate NULL 
 pointers.
 Indeed. I will commit that.

Wait. free_sysvstub_hashmapp does tolerate NULL pointers.

hashmap_steal_first will return NULL if the hashmap is NULL. And
hashmap_free is fine with NULL too. Your patch makes it more obvious
that free_sysvstub_hashmapp does tolerate NULL but destructors should
tolerate NULL as per the coding style. So I guess it should just be
assumed? I will leave it up to the others to decide what the best
style is here.
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [RFC][PATCH 1/2] libsystemd: add sd-device library

[Thomas H.P. Andersen]

On Wed, Apr 1, 2015 at 3:00 PM, Tom Gundersen t...@jklm.no wrote:
 This provides equivalent functionality to libudev-device, but in the
 systemd style. The public API only caters to creating sd_device objects
 from for devices that already exist in /sys, there is no support for
 listening for monitoring uevents or creating devices received over
 the udev netlink protocol.

 The private API contains the necessary functionality to make sd-device
 a drop-in replacement for libudev-device, but which we will not export
 publicly.
 ---

 Hi guys,

 This is another step on the way of ripping out the bits of libudev that will
 still be useful once the udev netlink transport has been replaced by kdbus.

 Feedback welcome.

Not a real review. Just a few nitpicks.


  Makefile.am|9 +-
  src/libsystemd/sd-device/device-internal.h |  125 ++
  src/libsystemd/sd-device/device-private.c  | 1101 +
  src/libsystemd/sd-device/device-private.h  |   63 +
  src/libsystemd/sd-device/device-util.h |   48 +
  src/libsystemd/sd-device/sd-device.c   | 1812 
 
  src/systemd/sd-device.h|   77 ++
  7 files changed, 3234 insertions(+), 1 deletion(-)
  create mode 100644 src/libsystemd/sd-device/device-internal.h
  create mode 100644 src/libsystemd/sd-device/device-private.c
  create mode 100644 src/libsystemd/sd-device/device-private.h
  create mode 100644 src/libsystemd/sd-device/device-util.h
  create mode 100644 src/libsystemd/sd-device/sd-device.c
  create mode 100644 src/systemd/sd-device.h

 diff --git a/Makefile.am b/Makefile.am
 index 93fdbc2..9509247 100644
 --- a/Makefile.am
 +++ b/Makefile.am
 @@ -231,6 +231,7 @@ AM_CPPFLAGS = \
 -I $(top_srcdir)/src/libsystemd/sd-rtnl \
 -I $(top_srcdir)/src/libsystemd/sd-network \
 -I $(top_srcdir)/src/libsystemd/sd-hwdb \
 +   -I $(top_srcdir)/src/libsystemd/sd-device \
 -I $(top_srcdir)/src/libsystemd-network \
 -I $(top_srcdir)/src/libsystemd-terminal \
 $(OUR_CPPFLAGS)
 @@ -2918,6 +2919,7 @@ libsystemd_internal_la_SOURCES = \
 src/systemd/sd-path.h \
 src/systemd/sd-network.h \
 src/systemd/sd-hwdb.h \
 +   src/systemd/sd-device.h \
 src/libsystemd/sd-bus/sd-bus.c \
 src/libsystemd/sd-bus/bus-control.c \
 src/libsystemd/sd-bus/bus-control.h \
 @@ -2981,7 +2983,12 @@ libsystemd_internal_la_SOURCES = \
 src/libsystemd/sd-network/network-util.c \
 src/libsystemd/sd-hwdb/sd-hwdb.c \
 src/libsystemd/sd-hwdb/hwdb-util.h \
 -   src/libsystemd/sd-hwdb/hwdb-internal.h
 +   src/libsystemd/sd-hwdb/hwdb-intenal.h \
 +   src/libsystemd/sd-device/device-internal.h \
 +   src/libsystemd/sd-device/device-util.h \
 +   src/libsystemd/sd-device/sd-device.c \
 +   src/libsystemd/sd-device/device-private.c \
 +   src/libsystemd/sd-device/device-private.h

  nodist_libsystemd_internal_la_SOURCES = \
 src/libsystemd/libsystemd.sym
 diff --git a/src/libsystemd/sd-device/device-internal.h 
 b/src/libsystemd/sd-device/device-internal.h
 new file mode 100644
 index 000..59ec1a6
 --- /dev/null
 +++ b/src/libsystemd/sd-device/device-internal.h
 @@ -0,0 +1,125 @@
 +/***
 +  This file is part of systemd.
 +
 +  Copyright 2008-2012 Kay Sievers k...@vrfy.org
 +  Copyright 2014 Tom Gundersen t...@jklm.no
 +
 +  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/.
 +***/
 +
 +#pragma once
 +
 +#include hashmap.h
 +#include set.h
 +
 +struct sd_device {
 +uint64_t n_ref;
 +
 +sd_device *parent;
 +bool parent_set; /* no need to try to reload parent */
 +
 +OrderedHashmap *properties;
 +Iterator properties_iterator;
 +uint64_t properties_generation; /* changes whenever the properties 
 are changed */
 +uint64_t properties_iterator_generation; /* generation when 
 iteration was started */
 +
 +/* the subset of the properties that should be written to the db*/
 +OrderedHashmap *properties_db;
 +
 +Hashmap *sysattr_values; /* cached sysattr values */
 +
 +Set *sysattrs; /* names of sysattrs */
 +Iterator sysattrs_iterator;
 +bool sysattrs_read; /* don't try to re-read sysattrs once read */
 +
 +Set *tags;
 +Iterator tags_iterator;
 +

Re: [systemd-devel] Removing unnecessary includes

[Thomas H.P. Andersen]

On Mon, Feb 23, 2015 at 2:02 PM, Lennart Poettering
mzerq...@0pointer.de wrote:
 On Sun, 22.02.15 14:56, Thomas H.P. Andersen (pho...@gmail.com) wrote:

  For info I am attaching a diff with the changes so far: 1309 includes
  removed out of the current 7707. It is only compile and make
  check-tested. I am only looking for comments (and perhaps compile
  testing on an AppArmor system as I do not have a system with that).

 The patch bitrots quickly. I have updated it and also dropped a few
 removals of architecture.h and endian.h. It passes make check and
 I have not seen any issues with using it on my own system. Still not
 tested with AppArmor though.

 If you are reasonably sure that the patch is OK, go ahead and push. We
 are still at the beginning of the cycle, and can fix any fallout
 in time.

Ok. I pushed the patch now.
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] Removing unnecessary includes

[Thomas H.P. Andersen]

On Sun, Feb 15, 2015 at 11:53 PM, Thomas H.P. Andersen pho...@gmail.com wrote:
 On Tue, Feb 10, 2015 at 10:05 PM, Lennart Poettering
 lenn...@poettering.net wrote:
 On Sat, 07.02.15 10:29, Thomas H.P. Andersen (pho...@gmail.com) wrote:

 Hi,

 I am looking at ways to automatically trim the unnecessary includes.
 One way to do it is a script[1] which simply tests if the compile
 still works after removing each include one at a time. It does this in
 reverse order for all includes in the .c files. Using -Werror we catch
 any new warnings too.

 I think this is quite useful, but I'd also be really careful with
 this. glibc versions sometimes require different headers to be
 included to get some functionality, thus automatic removal of headers
 that are unnecessary on one system doesn't mean this is universally
 the case... Moreover depdending on compile-time options you might
 different headers...

 I have used IWYU to only drop headers that we do not use any symbols
 from. There are no automatically added headers nor includes replaced
 by forward declarations.I have manually checked all removals from
 files that contain a #ifdef or #if defined() to catch issues from
 various compile-time option combinations. No includes of missing.h
 were removed and I tried to be careful with endianness.

 Are there specific headers I should be extra careful with or ignore 
 completely?

 For info I am attaching a diff with the changes so far: 1309 includes
 removed out of the current 7707. It is only compile and make
 check-tested. I am only looking for comments (and perhaps compile
 testing on an AppArmor system as I do not have a system with that).

The patch bitrots quickly. I have updated it and also dropped a few
removals of architecture.h and endian.h. It passes make check and
I have not seen any issues with using it on my own system. Still not
tested with AppArmor though.

I moved the patch to github:
https://github.com/phomes/systemd-1/commit/cf3c313747ebae18b63effb251f801ac4c370f05

 Here is a list the headers removed and number of times removed:
 74 sys/types.h
 69 unistd.h
 65 string.h
 63 assert.h
 53 util.h
 45 fcntl.h
 42 stdlib.h
 38 inttypes.h
 29 errno.h
 28 sys/stat.h
 21 strv.h
 19 label.h
 17 sys/socket.h
 16 path-util.h
 16 fileio.h
 16 def.h
 15 stdio.h
 15 dirent.h
 15 arpa/inet.h
 14 unit.h
 14 mkdir.h
 14 log.h
 14 ctype.h
 13 sys/un.h
 13 dbus-unit.h
 12 stdarg.h
 12 set.h
 12 poll.h
 12 netinet/ether.h
 11 limits.h
 11 bus-message.h
 10 stdbool.h
 10 signal.h
  9 time.h
  9 sys/wait.h
  9 socket-util.h
  9 sd-bus.h
  9 pwd.h
  9 network-internal.h
  9 list.h
  8 sys/ioctl.h
  8 sd-id128.h
  8 net/if.h
  8 load-fragment.h
  8 getopt.h
  7 sys/param.h
  7 macro.h
  7 hashmap.h
  7 event-util.h
  7 bus-error.h
  6 utf8.h
  6 sys/time.h
  6 sys/prctl.h
  6 grp.h
  6 conf-parser.h
  6 capability.h
  6 bus-util.h
  6 build.h
  5 udev-util.h
  5 sys/timex.h
  5 sys/timerfd.h
  5 sys/syscall.h
  5 sys/epoll.h
  5 load-dropin.h
  4 virt.h
  4 termios.h
  4 sys/signalfd.h
  4 sys/mount.h
  4 stddef.h
  4 sd-messages.h
  4 resolved-manager.h
  4 netinet/in.h
  4 manager.h
  4 logind-seat.h
  4 linux/vt.h
  4 libudev.h
  4 exit-status.h
  4 execute.h
  4 conf-files.h
  4 cgroup-util.h
  4 byteswap.h
  4 bus-control.h
  4 bus-common-errors.h
  4 audit.h
  3 time-util.h
  3 sys/utsname.h
  3 sys/resource.h
  3 stdint.h
  3 special.h
  3 smack-util.h
  3 resolved-dns-scope.h
  3 net/ethernet.h
  3 logind-session.h
  3 logind.h
  3 libudev.h
  3 in-addr-util.h
  3 endian.h
  2 wchar.h
  2 unit-name.h
  2 sys/xattr.h
  2 systemd/sd-login.h
  2 systemd/sd-journal.h
  2 sys/statvfs.h
  2 sys/mman.h
  2 synthesize.h
  2 siphash24.h
  2 sd-rtnl.h
  2 sd-event.h
  2 sd-dhcp-client.h
  2 sd-bus-protocol.h
  2 rtnl-util.h
  2 rtnl-internal.h
  2 resolved-dns-stream.h
  2 resolved-dns-server.h
  2 resolved-dns-rr.h
  2 network-util.h
  2 netinet/if_ether.h
  2 mount-setup.h
  2 mount.h
  2 logind-device.h
  2 linux/types.h
  2 linux/limits.h
  2 linux/ioctl.h
  2 linux/fs.h
  2 libudev-private.h
  2 journal-authenticate.h
  2 install.h
  2 fsprg.h
  2 fnmatch.h
  2 failure-action.h
  2 driver.h
  2 dhcp-lease-internal.h
  2 dbus-kill.h
  2 clock-util.h
  2 cgroup.h
  2 bus-label.h
  2 asm/types.h
  2 arpa/nameser.h
  1 xml.h
  1 xkbcommon/xkbcommon.h
  1 unaligned.h
  1 target.h
  1 sys/vfs.h
  1 sys/uio.h
  1 sys/swap.h
  1 sys/select.h
  1 sys/inotify.h
  1 sys/file.h
  1 sys/eventfd.h
  1 strxcpyx.h
  1 specifier.h
  1 service.h
  1 sd-lldp.h
  1 sd-dhcp-lease.h
  1 sd-daemon.h
  1 resolved-dns-transaction.h
  1 resolved-dns-query.h
  1 resolved-dns-domain.h
  1 pthread.h
  1 path-lookup.h
  1 networkd-netdev.h
  1 networkd-link.h
  1 mntent.h
  1 machine.h
  1 logind-user.h
  1 logind-session-device.h
  1 logind-inhibit.h
  1 locale.h
  1 linux/veth.h
  1 linux/sched.h
  1 linux/ppp_defs.h
  1 linux/oom.h
  1 linux/netlink.h
  1 linux/input.h
  1 linux/in6.h
  1 linux/if_link.h
  1 linux/if.h
  1 linux/if_ether.h

Re: [systemd-devel] Removing unnecessary includes

[Thomas H.P. Andersen]

On Wed, Feb 11, 2015 at 12:39 PM, Lennart Poettering
lenn...@poettering.net wrote:
 On Wed, 11.02.15 01:40, Thomas H.P. Andersen (pho...@gmail.com) wrote:

 Yep. Makes sense.

 Here is a status on what I have done so far.

 include-what-you-want does the following:
 1) sorts the includes
 2) adds missing headers for any symbols used
 3) adds forward declarations
 4) removes any unused headers (after step 2+3)
 5) changes some headers. (only saw sys/poll.h to poll.h for now)

 The diff we get out of that is too big a mess to locate what we want:
 the currently unused headers. To break it up I first did the sorting
 in separate step. (I have a patch to commit after 219 for some minor
 issues that came up from that). I then started to look at all the
 removals and one by one see if they make sense today, or was due to
 step 2/3/5, or was something we want to keep like missing.h. It is
 slow manual work but I will get there.

 It would be helpful to know if we might want 2, 3, and 5 done?

 What precisely do you mean by 3?

E.g. for src/journal/mmap-cache.h it replaces
#include sys/stat.h
by
struct stat;

since we only use the struct but none of the functions. Saves time
including that header and anything it brings in with it.

 Don't care too much about 2 either way. 5 sounds useful.

Will include 5.

 Lennart

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

Re: [systemd-devel] Removing unnecessary includes

[Thomas H.P. Andersen]

On Tue, Feb 10, 2015 at 10:13 PM, Lennart Poettering
lenn...@poettering.net wrote:
 On Mon, 09.02.15 10:19, Thomas H.P. Andersen (pho...@gmail.com) wrote:

 include-what-you-use is actually pretty nice. It is also a little bit
 crazy. It wants to include everything directly and we would add a lot
 includes for errno.h, string.h, stdlib.h, stdbool.h, stddef.h, etc etc
 everywhere. It also wants to use forward declares when possible. The
 coding style does not say anything specific about this. Any
 preferences? I will use the tool to trim the unnecessary includes
 first and leave its other advise for later. It will be interesting to
 see how each step affects the build time.

 A feature is that I find interesting is that it can comment each
 include with the reason that it was included. Not something to commit,
 but useful to get an overview of why each include is there.

 Hmm, I find this unnecessary noise I must say...

Well, for someone who is still learning it can be useful :) Getting
that list of symbols actually used from each header adds some context
and makes the includes feel less magic. Anyway, something to use on
the fly but not to commit of course. There is an option to turn it off
so that is not a problem.

 #include stdbool.h// for false, true, bool
 #include stddef.h // for offsetof, size_t
 #include stdint.h // for uint64_t
 #include string.h // for strdup, strlen
 #include config.h // for PACKAGE_STRING,
 #VERSION

 This one we get though -include config.h on the gcc command line.

We can configure it to understand that.

 #include src/shared/macro.h   // for assert, assert_cc, etc
 #include src/shared/path-lookup.h
 #include src/shared/time-util.h   // for format_timespan, usec_t, etc
 #include src/systemd/sd-bus-protocol.h  // for ::SD_BUS_TYPE_ARRAY

 Nah, please no absolute includes...

 Any chance this can turned off?

I did not find any option for that but it is something to look into.

 The full include-list for src/analyze/analyze.c:
 #include errno.h  // for EIO, ENOMEM, E2BIG, EINVAL, 
 etc
 #include fnmatch.h// for fnmatch
 #include getopt.h // for optind, no_argument, optarg, 
 etc
 #include locale.h // for NULL, setlocale, LC_ALL, etc
 #include stdbool.h// for false, true, bool
 #include stddef.h // for offsetof, size_t
 #include stdint.h // for uint64_t
 #include stdio.h  // for printf, puts, fputs, stdout
 #include stdlib.h // for free, qsort, EXIT_FAILURE, etc
 #include string.h // for strdup, strlen
 #include analyze-verify.h // for verify_units
 #include build.h  // for SYSTEMD_FEATURES
 #include bus-error.h  // for bus_error_message
 #include bus-util.h   // for UnitInfo, etc
 #include config.h // for PACKAGE_STRING, VERSION
 #include hashmap.h// for hashmap_get, Hashmap, etc
 #include log.h// for log_error, log_oom, etc
 #include pager.h  // for pager_close, pager_open
 #include sd-bus.h // for sd_bus, SD_BUS_ERROR_NULL, etc
 #include special.h// for SPECIAL_DEFAULT_TARGET
 #include src/shared/macro.h   // for assert, assert_cc, etc
 #include src/shared/path-lookup.h
 #include src/shared/time-util.h   // for format_timespan, usec_t, etc
 #include src/systemd/sd-bus-protocol.h  // for ::SD_BUS_TYPE_ARRAY
 #include strv.h   // for strv_isempty, STRV_FOREACH, 
 etc
 #include strxcpyx.h   // for strpcpyf
 #include unit-name.h  // for unit_dbus_path_from_name
 #include util.h   // for isempty, streq, etc

 Note that I actually try to roughly maintain an order when including
 things: For the headers of other projects i try to put system headers
 first, 3rd party headers second. For our own stuff I try to put
 external API stuff first (i.e. sd-*.h), followed by internal API-like
 stuff form src/shared, and finally the other headers that are prviate
 to the specific module the code is in.

 I'd really like to avoid a scheme where this is reordered
 randomly...

Understood. The include-what-you-use tool consists of two separate
parts. The first is a program to scan the code and create a report of
what to add and remove. The second part is a python script that reads
the report and patches the source files. Perhaps the latter be
extended to do what we need.

 The order kinda is relevant, since more local, specific
 definitions should never influence more public, generic
 interfaces, if you follow what I mean? A lot of header files use
 #ifndef, and only conditionally define things then. They should not
 get

Re: [systemd-devel] Removing unnecessary includes

[Thomas H.P. Andersen]

On Sat, Feb 7, 2015 at 3:38 PM, Thomas H.P. Andersen pho...@gmail.com wrote:
 On Sat, Feb 7, 2015 at 2:37 PM, Ronny Chevalier
 chevalier.ro...@gmail.com wrote:
 2015-02-07 14:05 GMT+01:00 Daniele Nicolodi dani...@grinta.net:
 On 07/02/15 10:29, Thomas H.P. Andersen wrote:
 I am looking at ways to automatically trim the unnecessary includes.
 One way to do it is a script[1] which simply tests if the compile
 still works after removing each include one at a time. It does this in
 reverse order for all includes in the .c files. Using -Werror we catch
 any new warnings too.

 Hello Thomas,

 this approach is not correct: in this way each source file would not be
 required to include the headers included by other files included before.
 For example, if header file a.h includes shared.h and implementation
 file requires the definitions of a.h and shared.h, only the first
 dependency would be detected by this method.

 However, it is good practice to include all the required header files,
 whether those are already included by others or not.


 Hi,

 I agree with Daniele. If you want to include the proper headers in
 each file maybe you can use include-what-you-use [0], but this is a
 rather recent project with lots of issues that will force you to do a
 lots of manual review.

 [0] https://code.google.com/p/include-what-you-use/

 Looks useful. Will take a look.

include-what-you-use is actually pretty nice. It is also a little bit
crazy. It wants to include everything directly and we would add a lot
includes for errno.h, string.h, stdlib.h, stdbool.h, stddef.h, etc etc
everywhere. It also wants to use forward declares when possible. The
coding style does not say anything specific about this. Any
preferences? I will use the tool to trim the unnecessary includes
first and leave its other advise for later. It will be interesting to
see how each step affects the build time.

A feature is that I find interesting is that it can comment each
include with the reason that it was included. Not something to commit,
but useful to get an overview of why each include is there.

Include-what-you-use is not packaged yet in fedora and you need to
edit the cmake files to make it build there. Dave Johansen is working
on the packaging though and it looks like it could go in any day now.
https://bugzilla.redhat.com/show_bug.cgi?id=1091659

To run it you need something like:
./autogen.sh c  make -k CC=include-what-you-use

Here is an example of the advise it gives for a single file:

src/analyze/analyze.c should add these lines:
#include errno.h  // for EIO, ENOMEM, E2BIG, EINVAL, etc
#include stdbool.h// for false, true, bool
#include stddef.h // for offsetof, size_t
#include stdint.h // for uint64_t
#include string.h // for strdup, strlen
#include config.h // for PACKAGE_STRING, VERSION
#include src/shared/macro.h   // for assert, assert_cc, etc
#include src/shared/path-lookup.h
#include src/shared/time-util.h   // for format_timespan, usec_t, etc
#include src/systemd/sd-bus-protocol.h  // for ::SD_BUS_TYPE_ARRAY

src/analyze/analyze.c should remove these lines:
- #include sys/utsname.h  // lines 27-27
- #include fileio.h  // lines 38-38
- #include install.h  // lines 33-33

The full include-list for src/analyze/analyze.c:
#include errno.h  // for EIO, ENOMEM, E2BIG, EINVAL, etc
#include fnmatch.h// for fnmatch
#include getopt.h // for optind, no_argument, optarg, etc
#include locale.h // for NULL, setlocale, LC_ALL, etc
#include stdbool.h// for false, true, bool
#include stddef.h // for offsetof, size_t
#include stdint.h // for uint64_t
#include stdio.h  // for printf, puts, fputs, stdout
#include stdlib.h // for free, qsort, EXIT_FAILURE, etc
#include string.h // for strdup, strlen
#include analyze-verify.h // for verify_units
#include build.h  // for SYSTEMD_FEATURES
#include bus-error.h  // for bus_error_message
#include bus-util.h   // for UnitInfo, etc
#include config.h // for PACKAGE_STRING, VERSION
#include hashmap.h// for hashmap_get, Hashmap, etc
#include log.h// for log_error, log_oom, etc
#include pager.h  // for pager_close, pager_open
#include sd-bus.h // for sd_bus, SD_BUS_ERROR_NULL, etc
#include special.h// for SPECIAL_DEFAULT_TARGET
#include src/shared/macro.h   // for assert, assert_cc, etc
#include src/shared/path-lookup.h
#include src/shared/time-util.h   // for format_timespan, usec_t, etc
#include src/systemd/sd-bus-protocol.h  // for ::SD_BUS_TYPE_ARRAY
#include strv.h

Re: [systemd-devel] Removing unnecessary includes

[Thomas H.P. Andersen]

On Sat, Feb 7, 2015 at 2:37 PM, Ronny Chevalier
chevalier.ro...@gmail.com wrote:
 2015-02-07 14:05 GMT+01:00 Daniele Nicolodi dani...@grinta.net:
 On 07/02/15 10:29, Thomas H.P. Andersen wrote:
 I am looking at ways to automatically trim the unnecessary includes.
 One way to do it is a script[1] which simply tests if the compile
 still works after removing each include one at a time. It does this in
 reverse order for all includes in the .c files. Using -Werror we catch
 any new warnings too.

 Hello Thomas,

 this approach is not correct: in this way each source file would not be
 required to include the headers included by other files included before.
 For example, if header file a.h includes shared.h and implementation
 file requires the definitions of a.h and shared.h, only the first
 dependency would be detected by this method.

 However, it is good practice to include all the required header files,
 whether those are already included by others or not.


 Hi,

 I agree with Daniele. If you want to include the proper headers in
 each file maybe you can use include-what-you-use [0], but this is a
 rather recent project with lots of issues that will force you to do a
 lots of manual review.

 [0] https://code.google.com/p/include-what-you-use/

Looks useful. Will take a look.

 Cheers,
 Daniele

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

Re: [systemd-devel] Removing unnecessary includes

[Thomas H.P. Andersen]

On Sat, Feb 7, 2015 at 2:05 PM, Daniele Nicolodi dani...@grinta.net wrote:
 On 07/02/15 10:29, Thomas H.P. Andersen wrote:
 I am looking at ways to automatically trim the unnecessary includes.
 One way to do it is a script[1] which simply tests if the compile
 still works after removing each include one at a time. It does this in
 reverse order for all includes in the .c files. Using -Werror we catch
 any new warnings too.

 Hello Thomas,

 this approach is not correct: in this way each source file would not be
 required to include the headers included by other files included before.
 For example, if header file a.h includes shared.h and implementation
 file requires the definitions of a.h and shared.h, only the first
 dependency would be detected by this method.

 However, it is good practice to include all the required header files,
 whether those are already included by others or not.

Oh, I did not mean to use the output of deheader directly. Only as a
way to find potential targets for removal. Each removal should be
reviewed manually of course.

 Cheers,
 Daniele

 ___
 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

Re: [systemd-devel] [systemd-commits] src/bus-proxyd src/login src/machine src/nspawn src/tmpfiles

[Thomas H.P. Andersen]

On Tue, Feb 3, 2015 at 12:50 AM, Zbigniew Jędrzejewski-Szmek
zbys...@in.waw.pl wrote:
 On Mon, Feb 02, 2015 at 02:07:37PM -0800, Thomas H.P. Andersen wrote:
 --- a/src/nspawn/nspawn.c
 +++ b/src/nspawn/nspawn.c
 @@ -3610,7 +3610,6 @@ int main(int argc, char *argv[]) {
  }

  if (arg_ephemeral) {
 -_cleanup_release_lock_file_ LockFile original_lock 
 = LOCK_FILE_INIT;
  char *np;

  /* If the specified path is a mount point we
 diff --git a/src/tmpfiles/tmpfiles.c b/src/tmpfiles/tmpfiles.c
 index 930b9a6..443851a 100644
 --- a/src/tmpfiles/tmpfiles.c
 +++ b/src/tmpfiles/tmpfiles.c
 @@ -630,7 +630,7 @@ static int get_xattrs_from_arg(Item *i) {

  while ((r = unquote_first_word(p, xattr, false))  0) {
  _cleanup_free_ char *tmp = NULL, *name = NULL,
 -*value = NULL, *value2 = NULL, *_xattr = xattr;
 +*value = NULL, *value2 = NULL;
 This leaks xattr, no?
oh, you are right. I will revert that.

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

Re: [systemd-devel] [PATCH] initial sysv-generator test suite

[Thomas H.P. Andersen]

On Tue, Jan 20, 2015 at 8:08 PM, Martin Pitt martin.p...@ubuntu.com wrote:
 Hey Jóhann,

 Jóhann B. Guðmundsson [2015-01-20 17:55 +]:
 We only provide backwards compatibility with initscript which are lsb
 compliance and I dont think  .something ending on a script confirms to
 that standard hence that test should be unnecessary and that initscript be
 fixed upstream as in that .something ending removed  ( or better yet that
 initscript be migrated )

 But the generator already handles .sh extensions, as they do exist in
 the wild (and being compatible to them is the whole point of the
 generator -- for new things you'd write a proper unit right away).
 It's just missing that .sh extension handling in that particular
 place, and as it's (correctly) doing it in another you get that bug.

Initscripts ending with .sh was indeed a case handled by the
sysv-generator from the beginning. I think that fixing this specific
issue makes sense and the patch (in the other email) looks pretty
simple to me.

Thanks for writing all these tests Martin. The bugs found in the
generator are embarrassing and in hindsight I should have tested this
more thoroughly on a debian system instead of just fedora. I look
forward to all your tests of this. Let me know if I can help out with
extending the test cases or anything.

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

Re: [systemd-devel] [systemd-commits] 2 commits - src/core src/journal-remote src/network

[Thomas H.P. Andersen]

On Sat, Dec 13, 2014 at 1:26 AM, Zbigniew Jędrzejewski-Szmek
zbys...@in.waw.pl wrote:
 On Fri, Dec 12, 2014 at 12:58:04PM -0800, Thomas H.P. Andersen wrote:
 Author: Thomas Hindoe Paaboel Andersen pho...@gmail.com
 Date:   Fri Dec 12 19:51:41 2014 +0100

 wrap a few *_FOREACH macros in curly braces

 cppcheck would give up with syntax error without them. This led
 to reports of syntax errors in unrelated locations and potentially
 hid other errors

 cppcheck is full of errors anyway. I don't think we should make the code
 less pretty just to satisfy a checker, and a rarely used one.

It is a tradeoff of course. It is no problem for me if we revert it.
We just have a make target for cppcheck so I thought I would make it
run without syntax errors. Cppcheck finds a lot of false positives but
there are real issues in the results as well. Like the llvm static
analyzer I think it is a useful tool to check new code.

- Thomas



 diff --git a/src/core/load-fragment.c b/src/core/load-fragment.c
 index 3fbe680..8e5be87 100644
 --- a/src/core/load-fragment.c
 +++ b/src/core/load-fragment.c
 @@ -3586,7 +3586,7 @@ int unit_load_fragment(Unit *u) {
  return r;

  /* Try to find an alias we can load this with */
 -if (u-load_state == UNIT_STUB)
 +if (u-load_state == UNIT_STUB) {
  SET_FOREACH(t, u-names, i) {

  if (t == u-id)
 @@ -3599,6 +3599,7 @@ int unit_load_fragment(Unit *u) {
  if (u-load_state != UNIT_STUB)
  break;
  }
 +}

  /* And now, try looking for it under the suggested (originally 
 linked) path */
  if (u-load_state == UNIT_STUB  u-fragment_path) {
 @@ -3628,7 +3629,7 @@ int unit_load_fragment(Unit *u) {
  if (r  0)
  return r;

 -if (u-load_state == UNIT_STUB)
 +if (u-load_state == UNIT_STUB) {
  SET_FOREACH(t, u-names, i) {
  _cleanup_free_ char *z = NULL;

 @@ -3646,6 +3647,7 @@ int unit_load_fragment(Unit *u) {
  if (u-load_state != UNIT_STUB)
  break;
  }
 +}
  }

  return 0;
 diff --git a/src/journal-remote/journal-remote.c 
 b/src/journal-remote/journal-remote.c
 index 6ec5ad2..5050616 100644
 --- a/src/journal-remote/journal-remote.c
 +++ b/src/journal-remote/journal-remote.c
 @@ -1469,13 +1469,13 @@ static int setup_gnutls_logger(char **categories) {

  gnutls_global_set_log_function(log_func_gnutls);

 -if (categories)
 +if (categories) {
  STRV_FOREACH(cat, categories) {
  r = log_enable_gnutls_category(*cat);
  if (r  0)
  return r;
  }
 -else
 +} else
  log_reset_gnutls_level();
  }
  #endif

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

Re: [systemd-devel] [PATCH 2/2] tty-ask-password-agent: fix CID 996261

[Thomas H.P. Andersen]

On Mon, Nov 17, 2014 at 7:54 PM, Greg KH gre...@linuxfoundation.org wrote:
 On Tue, Nov 18, 2014 at 12:21:29AM +0530, Susant Sahani wrote:
 On 11/18/2014 12:06 AM, Greg KH wrote:
 On Mon, Nov 17, 2014 at 06:47:33PM +0100, Ronny Chevalier wrote:
 2014-11-17 18:31 GMT+01:00 Greg KH gre...@linuxfoundation.org:
 On Mon, Nov 17, 2014 at 10:44:14PM +0530, Susant Sahani wrote:
 On 11/17/2014 10:39 PM, Greg KH wrote:
 On Mon, Nov 17, 2014 at 10:36:53PM +0530, Susant Sahani wrote:
 On 11/17/2014 10:26 PM, Greg KH wrote:
 On Mon, Nov 17, 2014 at 04:28:58PM +0530, Susant Sahani wrote:
 ---
   src/tty-ask-password-agent/tty-ask-password-agent.c | 2 +-
   1 file changed, 1 insertion(+), 1 deletion(-)
 
 diff --git a/src/tty-ask-password-agent/tty-ask-password-agent.c 
 b/src/tty-ask-password-agent/tty-ask-password-agent.c
 index e6dc84b..1fc792b 100644
 --- a/src/tty-ask-password-agent/tty-ask-password-agent.c
 +++ b/src/tty-ask-password-agent/tty-ask-password-agent.c
 @@ -376,8 +376,8 @@ static int wall_tty_block(void) {
   return -ENOMEM;
 
   mkdir_parents_label(p, 0700);
 -mkfifo(p, 0600);
 
 +(void)mkfifo(p, 0600);
 
 You really aren't fixing anything in these patches, just merely
 papering over the Coverity issues.  Which is fine, if you really want 
 to
 do that, but don't think it's anything other than that...
 
 Yes my intention is to for coverity only Any way next line 'open' 
 handling
 the error case .
 
 I'm sorry, but I don't understand this sentance at all, can you rephrase
 it?
 
 
 Sorry let me rephrase it. This patch only for coverity . The next like of
 mkfifo is open .
 
 (void)mkfifo(p, 0600);
 fd = open(p, O_RDONLY|O_CLOEXEC|O_NONBLOCK|O_NOCTTY);
 if (fd  0)
  return -errno;
 
 and open is handling the failure.
 
 Then coverity should be fixed, don't paper over stupid bugs in tools for
 no reason.
 I disagree.
 
 Coverity can not infer this in any possible way. How can coverity
 infer that we do not care about the return value of mkfifo ?
 It really depends of the semantic here.
 
 Coverity is a semantic checker, why can't it be changed to determine
 if mkfifo() is followed by open() and an error check, that it is safe
 code?  It does this for lots of other common patterns.

 For now mkfifo/mkdir/ioctl coverity is not that smart or is it ?

 Talk to the coverity people.  Given that it is a closed source tool,
 that costs money, I am very loath to do anything to make it better,
 and I really don't like it forcing programs to work around its
 deficiencies.

 greg k-h

What coverity is complaining about in this CID is this:
Unchecked return value from library. Calling mkfifo() without
checking return value. This library function may fail and return an
error code.

We can choose to either make it explicit that we don't care about the
return value with this patch, or we can just mark the issue as
intentional in coverity to make it go away. The (void) is a bit ugly
imo but it does show that ignoring the return was a conscious
decision. A matter of taste I guess.

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

Re: [systemd-devel] [PATCH] bootchart: Do not try to access data for non-existing CPU's

[Thomas H.P. Andersen]

On Sun, Sep 28, 2014 at 5:12 PM,  philippedesw...@gmail.com wrote:
 From: Philippe De Swert philippe.desw...@jollamobile.com

 Cpu's are assigned normally, so starting at 0, so the MAX_CPU index will
 always be one smaller than the actual number.

 Found with Coverity.
 ---
  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 3099ff1..9ea1b27 100644
 --- a/src/bootchart/store.c
 +++ b/src/bootchart/store.c
 @@ -199,7 +199,7 @@ vmstat_next:

  if (strstr(key, cpu)) {
  r = safe_atoi((const char*)(key+3), c);
 -if (r  0 || c  MAXCPUS)
 +if (r  0 || c  MAXCPUS -1)
  /* Oops, we only have room for MAXCPUS data 
 */
  break;
  sampledata-runtime[c] = atoll(rt);
 --
 1.8.3.2


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

[systemd-devel] [PATCH 1/3] bootchart: parse userinput with safe_atoi

[Thomas H.P. Andersen]

From: Thomas Hindoe Paaboel Andersen pho...@gmail.com

Found by coverity. Fixes: CID#996409
---
 src/bootchart/store.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/src/bootchart/store.c b/src/bootchart/store.c
index ed683e8..3099ff1 100644
--- a/src/bootchart/store.c
+++ b/src/bootchart/store.c
@@ -192,12 +192,14 @@ vmstat_next:
 
 m = buf;
 while (m) {
+int r;
+
 if (sscanf(m, %s %*s %*s %*s %*s %*s %*s %s %s, key, rt, wt) 
 3)
 goto schedstat_next;
 
 if (strstr(key, cpu)) {
-c = atoi((const char*)(key+3));
-if (c  MAXCPUS)
+r = safe_atoi((const char*)(key+3), c);
+if (r  0 || c  MAXCPUS)
 /* Oops, we only have room for MAXCPUS data */
 break;
 sampledata-runtime[c] = atoll(rt);
-- 
2.1.0

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

[systemd-devel] [PATCH 3/3] bootchart: use 'n/a' if PRETTY_NAME is not found

[Thomas H.P. Andersen]

From: Thomas Hindoe Paaboel Andersen pho...@gmail.com

Spotted with coverity. If parsing both /etc/os-release and
/usr/lib/os-release fails then null would be passed on. The calls
to parse the two files are allowed to fail. A empty /etc may not
have had the /etc/os-release symlink restored yet and we just
try again in the loop. If for whatever reason that does not happen
then we now pass on 'n/a' instead of null.
---
 src/bootchart/bootchart.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/bootchart/bootchart.c b/src/bootchart/bootchart.c
index e127ad3..a359307 100644
--- a/src/bootchart/bootchart.c
+++ b/src/bootchart/bootchart.c
@@ -477,7 +477,7 @@ int main(int argc, char *argv[]) {
 exit (EXIT_FAILURE);
 }
 
-svg_do(build);
+svg_do(strna(build));
 
 fprintf(stderr, systemd-bootchart wrote %s\n, output_file);
 
-- 
2.1.0

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

[systemd-devel] [PATCH 2/3] bootchart: check return of strftime

[Thomas H.P. Andersen]

From: Thomas Hindoe Paaboel Andersen pho...@gmail.com

Found by coverity. Fixes: CID#996314 and #996312
---
 src/bootchart/bootchart.c | 14 --
 src/bootchart/svg.c   |  6 --
 2 files changed, 16 insertions(+), 4 deletions(-)

diff --git a/src/bootchart/bootchart.c b/src/bootchart/bootchart.c
index 8ef5ad1..e127ad3 100644
--- a/src/bootchart/bootchart.c
+++ b/src/bootchart/bootchart.c
@@ -389,7 +389,12 @@ int main(int argc, char *argv[]) {
 
 if (!of  (access(arg_output_path, R_OK|W_OK|X_OK) == 0)) {
 t = time(NULL);
-strftime(datestr, sizeof(datestr), %Y%m%d-%H%M, 
localtime(t));
+r = strftime(datestr, sizeof(datestr), %Y%m%d-%H%M, 
localtime(t));
+if (r = 0) {
+log_error(Failed to format time.);
+return EXIT_FAILURE;
+}
+
 snprintf(output_file, PATH_MAX, %s/bootchart-%s.svg, 
arg_output_path, datestr);
 of = fopen(output_file, we);
 }
@@ -457,7 +462,12 @@ int main(int argc, char *argv[]) {
 
 if (!of) {
 t = time(NULL);
-strftime(datestr, sizeof(datestr), %Y%m%d-%H%M, 
localtime(t));
+r = strftime(datestr, sizeof(datestr), %Y%m%d-%H%M, 
localtime(t));
+if (r = 0) {
+log_error(Failed to format time.);
+return EXIT_FAILURE;
+}
+
 snprintf(output_file, PATH_MAX, %s/bootchart-%s.svg, 
arg_output_path, datestr);
 of = fopen(output_file, we);
 }
diff --git a/src/bootchart/svg.c b/src/bootchart/svg.c
index 135883f..cb58246 100644
--- a/src/bootchart/svg.c
+++ b/src/bootchart/svg.c
@@ -162,7 +162,7 @@ static void svg_title(const char *build) {
 char *c;
 FILE *f;
 time_t t;
-int fd;
+int fd, r;
 struct utsname uts;
 
 /* grab /proc/cmdline */
@@ -196,7 +196,9 @@ static void svg_title(const char *build) {
 
 /* date */
 t = time(NULL);
-strftime(date, sizeof(date), %a, %d %b %Y %H:%M:%S %z, 
localtime(t));
+r = strftime(date, sizeof(date), %a, %d %b %Y %H:%M:%S %z, 
localtime(t));
+if (r = 0)
+fprintf(stderr, Failed to format time\n);
 
 /* CPU type */
 fd = openat(procfd, cpuinfo, O_RDONLY);
-- 
2.1.0

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

Re: [systemd-devel] [PATCH 1/3] bootchart: parse userinput with safe_atoi

[Thomas H.P. Andersen]

Hi,

I am sending these small fixes for issues found with coverity for
review. I think that they are good to commit but I am sending them
here anyway because I cannot test them. My attempts to boot with
init=/usr/lib/systemd/systemd-bootchart hangs while starting udev.
Both with master, master + my changes, and also with the version
installed with fedora 21.

- Thomas

On Fri, Sep 26, 2014 at 10:01 PM, Thomas H.P. Andersen pho...@gmail.com wrote:
 From: Thomas Hindoe Paaboel Andersen pho...@gmail.com

 Found by coverity. Fixes: CID#996409
 ---
  src/bootchart/store.c | 6 --
  1 file changed, 4 insertions(+), 2 deletions(-)

 diff --git a/src/bootchart/store.c b/src/bootchart/store.c
 index ed683e8..3099ff1 100644
 --- a/src/bootchart/store.c
 +++ b/src/bootchart/store.c
 @@ -192,12 +192,14 @@ vmstat_next:

  m = buf;
  while (m) {
 +int r;
 +
  if (sscanf(m, %s %*s %*s %*s %*s %*s %*s %s %s, key, rt, 
 wt)  3)
  goto schedstat_next;

  if (strstr(key, cpu)) {
 -c = atoi((const char*)(key+3));
 -if (c  MAXCPUS)
 +r = safe_atoi((const char*)(key+3), c);
 +if (r  0 || c  MAXCPUS)
  /* Oops, we only have room for MAXCPUS data 
 */
  break;
  sampledata-runtime[c] = atoll(rt);
 --
 2.1.0

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

Re: [systemd-devel] [PATCH 3/3] util : Remove dead code

[Thomas H.P. Andersen]

On Tue, Sep 16, 2014 at 11:27 PM,  philippedesw...@gmail.com wrote:
 From: Philippe De Swert philippedesw...@gmail.com

 We only break out of the pre-ceding loop into the rest of the code
 if fd is actually = 0. So the  0 check will never be true and
 not necessary.

 Found with Coverity. Fixes: CID#1237577

I pushed a fix for this one 20 minutes ago. (I also set the CID as
assigned to me but maybe we need a better way to coordinate?)

 ---
  src/shared/util.c | 3 ---
  1 file changed, 3 deletions(-)

 diff --git a/src/shared/util.c b/src/shared/util.c
 index 61d6680..30b0364 100644
 --- a/src/shared/util.c
 +++ b/src/shared/util.c
 @@ -1878,9 +1878,6 @@ int open_terminal(const char *name, int mode) {
  c++;
  }

 -if (fd  0)
 -return -errno;
 -
  r = isatty(fd);
  if (r  0) {
  safe_close(fd);
 --
 1.8.3.2

 ___
 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

[systemd-devel] [PATCH 1/2] sd-bus: sd_bus_message_get_errno should only return positive errno

[Thomas H.P. Andersen]

From: Thomas Hindoe Paaboel Andersen pho...@gmail.com

sd_bus_message_get_errno can currently return either a number of
different poitive errno values (from bus-error-mapping), or a negative
EINVAL if passed null as parameter.

The check for null parameter was introduced in 
40ca29a1370379d43e44c0ed425eecc7218dcbca
at the same as the function was renamed from bus_message_to_errno and
made public API. Before becoming public the function used to return
only negative values.

It is weird to have a function return both positive and negative errno
and it generally looks like a mistake. The function is guarded by the
--enable-kdbus flags so I wonder if we still have time to fix it up?
It does not have any documentation yet. However, except for a few details
it is just a convenient way to call sd_bus_error_get_errno which is documented
to return only positive errno.

This patch makes it return only positive errno and fixes up the two
calls to the function that tried to cope with both positive and negative
values.
---
 src/libsystemd/sd-bus/bus-message.c | 2 +-
 src/libsystemd/sd-bus/sd-bus.c  | 2 --
 src/network/networkd-link.c | 2 --
 3 files changed, 1 insertion(+), 5 deletions(-)

diff --git a/src/libsystemd/sd-bus/bus-message.c 
b/src/libsystemd/sd-bus/bus-message.c
index bfb14fc..1fa3ad2 100644
--- a/src/libsystemd/sd-bus/bus-message.c
+++ b/src/libsystemd/sd-bus/bus-message.c
@@ -5337,7 +5337,7 @@ int bus_header_message_size(struct bus_header *h, size_t 
*sum) {
 }
 
 _public_ int sd_bus_message_get_errno(sd_bus_message *m) {
-assert_return(m, -EINVAL);
+assert_return(m, EINVAL);
 
 if (m-header-type != SD_BUS_MESSAGE_METHOD_ERROR)
 return 0;
diff --git a/src/libsystemd/sd-bus/sd-bus.c b/src/libsystemd/sd-bus/sd-bus.c
index 33b65ab..28b993b 100644
--- a/src/libsystemd/sd-bus/sd-bus.c
+++ b/src/libsystemd/sd-bus/sd-bus.c
@@ -349,8 +349,6 @@ static int hello_callback(sd_bus *bus, sd_bus_message 
*reply, void *userdata, sd
 assert(reply);
 
 r = sd_bus_message_get_errno(reply);
-if (r  0)
-return r;
 if (r  0)
 return -r;
 
diff --git a/src/network/networkd-link.c b/src/network/networkd-link.c
index 9bf1a81..427f695 100644
--- a/src/network/networkd-link.c
+++ b/src/network/networkd-link.c
@@ -725,8 +725,6 @@ static int set_hostname_handler(sd_bus *bus, sd_bus_message 
*m, void *userdata,
 return 1;
 
 r = sd_bus_message_get_errno(m);
-if (r  0)
-r = -r;
 if (r  0)
 log_warning_link(link, Could not set hostname: %s,
  strerror(r));
-- 
2.1.0

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

[systemd-devel] [PATCH 2/2] terminal: sd_bus_error_get_errno returns positive errno

[Thomas H.P. Andersen]

From: Thomas Hindoe Paaboel Andersen pho...@gmail.com

The 3 calls to sd_bus_error_get_errno appear to expect a negative
return value.

This patch negates the returned value so it matches the other error
cases in the 3 functions where sd_bus_error_get_errno is used.
---
 src/libsystemd-terminal/sysview.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/src/libsystemd-terminal/sysview.c 
b/src/libsystemd-terminal/sysview.c
index 2083f5a..fde87d1 100644
--- a/src/libsystemd-terminal/sysview.c
+++ b/src/libsystemd-terminal/sysview.c
@@ -263,7 +263,7 @@ static int session_take_control_fn(sd_bus *bus,
 
 log_debug(sysview: %s: TakeControl failed: %s: %s,
   session-name, e-name, e-message);
-error = sd_bus_error_get_errno(e);
+error = -sd_bus_error_get_errno(e);
 } else {
 session-has_control = true;
 error = 0;
@@ -1195,7 +1195,7 @@ static int context_ld_list_seats_fn(sd_bus *bus,
 
 log_debug(sysview: ListSeats on logind failed: %s: %s,
   error-name, error-message);
-return sd_bus_error_get_errno(error);
+return -sd_bus_error_get_errno(error);
 }
 
 r = sd_bus_message_enter_container(reply, 'a', (so));
@@ -1247,7 +1247,7 @@ static int context_ld_list_sessions_fn(sd_bus *bus,
 
 log_debug(sysview: ListSessions on logind failed: %s: %s,
   error-name, error-message);
-return sd_bus_error_get_errno(error);
+return -sd_bus_error_get_errno(error);
 }
 
 r = sd_bus_message_enter_container(reply, 'a', (susso));
-- 
2.1.0

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

Re: [systemd-devel] [PATCH 0/5] Coverity fixes

[Thomas H.P. Andersen]

On Wed, Sep 10, 2014 at 11:20 AM,  philippedesw...@gmail.com wrote:
 From: Philippe De Swert philippedesw...@gmail.com

 Hi,

 Yesterday I finally got to revive the systemd Coverity project on
 scan.coverity.org Unfortunately to see the errors reported you need
 to sign up, but I will make sure to approve requests for seeing the
 bugs whenever they show up.

Hi Philippe,

I was wondering if it is possible to enable selinux for the scans? We
have a lot of reports of Logically dead code. They are mostly due to
selinux_unit_access_check() being defined as
#define selinux_unit_access_check(unit, message, permission, error) 0
when HAVE_SELINUX is not set. See e.g. CID#1237573

I can of course just mark them as false positive but it would be
better to have those code paths covered as well.

 Also I quickly made some quick fixes (although since it was late at night
 yesterday there might be a few that were too quick ;) )
 There is a lot of work to do reviewing the error reports (as some are
 probably false positives) and fixing them. About 500 errors and an error
 defect density of 2.5 atm.

 Cheers,

 Philippe

 Philippe De Swert (5):
   [use after free] Avoid using m-kdbus after freeing it.
   [use after free] pattern is already freed, so do not dereference it in
 the error print
   [uninitialized] No need to check if num is  0
   [memleak] Do not leak mmapped area when other memory allocations fail.
   [memleak] Actually unref the buscreds on failure.

  src/journal/coredumpctl.c   |  4 ++--
  src/journal/mmap-cache.c| 10 +++---
  src/libsystemd-terminal/idev-keyboard.c |  3 ---
  src/libsystemd/sd-bus/bus-message.c |  6 +++---
  src/libsystemd/sd-bus/sd-bus.c  |  4 +++-
  5 files changed, 15 insertions(+), 12 deletions(-)

 --
 1.8.3.2

 ___
 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

Re: [systemd-devel] [PATCH 0/5] Coverity fixes

[Thomas H.P. Andersen]

On Wed, Sep 10, 2014 at 11:20 AM,  philippedesw...@gmail.com wrote:
 From: Philippe De Swert philippedesw...@gmail.com

 Hi,

 Yesterday I finally got to revive the systemd Coverity project on
 scan.coverity.org Unfortunately to see the errors reported you need
 to sign up, but I will make sure to approve requests for seeing the
 bugs whenever they show up.

Nice! I regularly run llvm's static analyzer and fix the issues it
finds. It seems that coverity finds different/more issues. I have sent
a request to help review those bugs.

 Also I quickly made some quick fixes (although since it was late at night
 yesterday there might be a few that were too quick ;) )
 There is a lot of work to do reviewing the error reports (as some are
 probably false positives) and fixing them. About 500 errors and an error
 defect density of 2.5 atm.

 Cheers,

 Philippe

 Philippe De Swert (5):
   [use after free] Avoid using m-kdbus after freeing it.
   [use after free] pattern is already freed, so do not dereference it in
 the error print
   [uninitialized] No need to check if num is  0
   [memleak] Do not leak mmapped area when other memory allocations fail.
   [memleak] Actually unref the buscreds on failure.

  src/journal/coredumpctl.c   |  4 ++--
  src/journal/mmap-cache.c| 10 +++---
  src/libsystemd-terminal/idev-keyboard.c |  3 ---
  src/libsystemd/sd-bus/bus-message.c |  6 +++---
  src/libsystemd/sd-bus/sd-bus.c  |  4 +++-
  5 files changed, 15 insertions(+), 12 deletions(-)

 --
 1.8.3.2

 ___
 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

Re: [systemd-devel] [RFC] [PATCH 2/3] resume: add a tool to write a device node's major:minor to /sys/power/resume.

[Thomas H.P. Andersen]

On Sat, Aug 23, 2014 at 2:47 PM, Ivan Shapovalov intelfx...@gmail.com wrote:
 This can be used to initiate a resume from hibernation by path to a swap
 device containing the hibernation image.

 The respective templated unit is also added. It is instantiated using
 path to the desired resume device.
 ---
  Makefile-man.am  |  9 +
  Makefile.am  | 28 --
  man/systemd-res...@.service.xml  | 81 +++
  src/resume/Makefile  |  1 +
  src/resume/resume.c  | 82 
 
  units/systemd-res...@.service.in | 23 +++
  6 files changed, 220 insertions(+), 4 deletions(-)
  create mode 100644 man/systemd-res...@.service.xml
  create mode 12 src/resume/Makefile
  create mode 100644 src/resume/resume.c
  create mode 100644 units/systemd-res...@.service.in

 diff --git a/Makefile-man.am b/Makefile-man.am
 index 5c289dd..00daae2 100644
 --- a/Makefile-man.am
 +++ b/Makefile-man.am
 @@ -76,6 +76,8 @@ MANPAGES += \
 man/systemd-nspawn.1 \
 man/systemd-path.1 \
 man/systemd-remount-fs.service.8 \
 +   man/systemd-resume-generator.8 \
 +   man/systemd-resume@.service.8 \
 man/systemd-run.1 \
 man/systemd-shutdownd.service.8 \
 man/systemd-sleep.conf.5 \
 @@ -206,6 +208,7 @@ MANPAGES_ALIAS += \
 man/systemd-poweroff.service.8 \
 man/systemd-reboot.service.8 \
 man/systemd-remount-fs.8 \
 +   man/systemd-resume.8 \
 man/systemd-shutdown.8 \
 man/systemd-shutdownd.8 \
 man/systemd-shutdownd.socket.8 \
 @@ -311,6 +314,7 @@ man/systemd-kexec.service.8: man/systemd-halt.service.8
  man/systemd-poweroff.service.8: man/systemd-halt.service.8
  man/systemd-reboot.service.8: man/systemd-halt.service.8
  man/systemd-remount-fs.8: man/systemd-remount-fs.service.8
 +man/systemd-resume.8: man/systemd-resume@.service.8
  man/systemd-shutdown.8: man/systemd-halt.service.8
  man/systemd-shutdownd.8: man/systemd-shutdownd.service.8
  man/systemd-shutdownd.socket.8: man/systemd-shutdownd.service.8
 @@ -592,6 +596,9 @@ man/systemd-reboot.service.html: 
 man/systemd-halt.service.html
  man/systemd-remount-fs.html: man/systemd-remount-fs.service.html
 $(html-alias)

 +man/systemd-resume.html: man/systemd-res...@.service.html
 +   $(html-alias)
 +
  man/systemd-shutdown.html: man/systemd-halt.service.html
 $(html-alias)

 @@ -1626,6 +1633,8 @@ EXTRA_DIST += \
 man/systemd-readahead-replay.service.xml \
 man/systemd-remount-fs.service.xml \
 man/systemd-resolved.service.xml \
 +   man/systemd-resume-generator.xml \
 +   man/systemd-res...@.service.xml \
 man/systemd-rfk...@.service.xml \
 man/systemd-run.xml \
 man/systemd-shutdownd.service.xml \
 diff --git a/Makefile.am b/Makefile.am
 index e238cde..820d082 100644
 --- a/Makefile.am
 +++ b/Makefile.am
 @@ -371,13 +371,15 @@ rootlibexec_PROGRAMS = \
 systemd-sleep \
 systemd-bus-proxyd \
 systemd-socket-proxyd \
 -   systemd-update-done
 +   systemd-update-done \
 +   systemd-resume

  systemgenerator_PROGRAMS = \
 systemd-getty-generator \
 systemd-fstab-generator \
 systemd-system-update-generator \
 -   systemd-debug-generator
 +   systemd-debug-generator \
 +   systemd-resume-generator

  dist_bashcompletion_DATA = \
 shell-completion/bash/busctl \
 @@ -509,7 +511,8 @@ nodist_systemunit_DATA = \
 units/initrd-udevadm-cleanup-db.service \
 units/initrd-switch-root.service \
 units/systemd-nspawn@.service \
 -   units/systemd-update-done.service
 +   units/systemd-update-done.service \
 +   units/systemd-resume@.service

  dist_userunit_DATA = \
 units/user/basic.target \
 @@ -556,7 +559,8 @@ EXTRA_DIST += \
 units/initrd-udevadm-cleanup-db.service.in \
 units/initrd-switch-root.service.in \
 units/systemd-nsp...@.service.in \
 -   units/systemd-update-done.service.in
 +   units/systemd-update-done.service.in \
 +   units/systemd-res...@.service.in

  CLEANFILES += \
 units/console-shell.service.m4 \
 @@ -1930,6 +1934,14 @@ systemd_delta_LDADD = \
 libsystemd-shared.la

  # 
 --
 +systemd_resume_SOURCES = \
 +   src/resume/resume.c
 +
 +systemd_resume_LDADD = \
 +   libsystemd-internal.la \
 +   libsystemd-shared.la
 +
 +# 
 --
  systemd_getty_generator_SOURCES = \
 src/getty-generator/getty-generator.c

 @@ -1962,6 +1974,14 @@ systemd_system_update_generator_LDADD = \
 libsystemd-label.la \
 libsystemd-shared.la

 +# 
 --
 

Re: [systemd-devel] [RFC] [PATCHv2 3/3] resume-generator: add a generator for instantiating the resume unit.

[Thomas H.P. Andersen]

On Sat, Aug 23, 2014 at 8:59 PM, Ivan Shapovalov intelfx...@gmail.com wrote:
 resume-generator understands resume= kernel command line parameter and
 instantiates the systemd-resume@.service accordingly if it is passed.

 This enables resume from hibernation using device specified on the kernel
 command line, where the device path may point to an arbitrary udev-created
 symlink, not only /dev/sdXY which is understood by the in-kernel
 implementation.
 ---
  Makefile-man.am |  2 +
  man/kernel-command-line.xml | 13 -
  man/systemd-resume-generator.xml| 91 
  src/resume-generator/Makefile   |  1 +
  src/resume-generator/resume-generator.c | 93 
 +
  5 files changed, 199 insertions(+), 1 deletion(-)
  create mode 100644 man/systemd-resume-generator.xml
  create mode 12 src/resume-generator/Makefile
  create mode 100644 src/resume-generator/resume-generator.c

 diff --git a/Makefile-man.am b/Makefile-man.am
 index be19905..00daae2 100644
 --- a/Makefile-man.am
 +++ b/Makefile-man.am
 @@ -76,6 +76,7 @@ MANPAGES += \
 man/systemd-nspawn.1 \
 man/systemd-path.1 \
 man/systemd-remount-fs.service.8 \
 +   man/systemd-resume-generator.8 \
 man/systemd-resume@.service.8 \
 man/systemd-run.1 \
 man/systemd-shutdownd.service.8 \
 @@ -1632,6 +1633,7 @@ EXTRA_DIST += \
 man/systemd-readahead-replay.service.xml \
 man/systemd-remount-fs.service.xml \
 man/systemd-resolved.service.xml \
 +   man/systemd-resume-generator.xml \
 man/systemd-res...@.service.xml \
 man/systemd-rfk...@.service.xml \
 man/systemd-run.xml \
 diff --git a/man/kernel-command-line.xml b/man/kernel-command-line.xml
 index f244bfc..4bc6cee 100644
 --- a/man/kernel-command-line.xml
 +++ b/man/kernel-command-line.xml
 @@ -351,6 +351,16 @@
  /listitem
  /varlistentry

 +varlistentry
 +termvarnameresume=/varname/term
 +
 +listitem
 +paraEnables resume from hibernation
 +using the specified device. For
 +details, see
 +
 citerefentryrefentrytitlesystemd-resume-generator/refentrytitlemanvolnum8/manvolnum/citerefentry./para
 +/listitem
 +/varlistentry
  /variablelist

  /refsect1
 @@ -373,7 +383,8 @@

 citerefentryrefentrytitlesystemd-gpt-auto-generator/refentrytitlemanvolnum8/manvolnum/citerefentry,

 citerefentryrefentrytitlesystemd-modules-load.service/refentrytitlemanvolnum8/manvolnum/citerefentry,

 citerefentryrefentrytitlesystemd-backlight@.service/refentrytitlemanvolnum8/manvolnum/citerefentry,
 -  
 citerefentryrefentrytitlesystemd-rfkill@.service/refentrytitlemanvolnum8/manvolnum/citerefentry
 +  
 citerefentryrefentrytitlesystemd-rfkill@.service/refentrytitlemanvolnum8/manvolnum/citerefentry,
 +  
 citerefentryrefentrytitlesystemd-resume-generator/refentrytitlemanvolnum8/manvolnum/citerefentry
/para
  /refsect1

 diff --git a/man/systemd-resume-generator.xml 
 b/man/systemd-resume-generator.xml
 new file mode 100644
 index 000..7962534
 --- /dev/null
 +++ b/man/systemd-resume-generator.xml
 @@ -0,0 +1,91 @@
 +?xml version=1.0?
 +!--*-nxml-*--
 +!DOCTYPE refentry PUBLIC -//OASIS//DTD DocBook XML V4.2//EN 
 http://www.oasis-open.org/docbook/xml/4.2/docbookx.dtd;
 +!--
 +  This file is part of systemd.
 +
 +  Copyright 2014 Ivan Shapovalov
 +
 +  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/.
 +--
 +refentry id=systemd-resume-generator
 +
 +refentryinfo
 +titlesystemd-resume-generator/title
 +productnamesystemd/productname
 +
 +authorgroup
 +author
 +contribDeveloper/contrib
 +firstnameIvan/firstname
 + 

Re: [systemd-devel] [PATCH v2] bootchart: use NSEC_PER_SEC

[Thomas H.P. Andersen]

Maybe these are candidates for this as well?

src/bootchart/bootchart.c:355:interval = (1.0 / arg_hz) * 10.0;
src/bootchart/bootchart.c:413:elapsed = (sample_stop -
sampledata-sampletime) * 10.0;
src/bootchart/bootchart.c:416:newint_s =
(time_t)(timeleft / 10.0);
src/bootchart/bootchart.c:417:newint_ns =
(long)(timeleft - (newint_s * 10.0));
src/bootchart/svg.c:634:max / 1024.0 /
(interval / 10.0));
src/bootchart/svg.c:743:max / 1024.0 /
(interval / 10.0));
src/bootchart/svg.c:770:trt = trt / 10.0;
src/bootchart/svg.c:812:twt = twt / 10.0;
src/bootchart/svg.c:1064:prt = (rt /
10) / (ps-sample-sampledata-sampletime -
prev-sampledata-sampletime);
src/bootchart/svg.c:1065:wrt = (wt /
10) / (ps-sample-sampledata-sampletime -
prev-sampledata-sampletime);
src/bootchart/svg.c:1104:(ps-last-runtime -
ps-first-runtime) / 10.0,
src/bootchart/store.c:413:/ 10.0;
src/cgtop/cgtop.c:179:x = ((uint64_t)
ts.tv_sec * 10ULL + (uint64_t) ts.tv_nsec) -
src/cgtop/cgtop.c:180:((uint64_t)
g-cpu_timestamp.tv_sec * 10ULL + (uint64_t)
g-cpu_timestamp.tv_nsec);
src/cgtop/cgtop.c:264:x = ((uint64_t)
ts.tv_sec * 10ULL + (uint64_t) ts.tv_nsec) -
src/cgtop/cgtop.c:265:((uint64_t)
g-io_timestamp.tv_sec * 10ULL + (uint64_t)
g-io_timestamp.tv_nsec);
src/cgtop/cgtop.c:271:g-io_input_bps
= (yr * 10ULL) / x;
src/cgtop/cgtop.c:272:g-io_output_bps
= (yw * 10ULL) / x;


On Mon, Aug 18, 2014 at 8:59 PM, Ronny Chevalier
chevalier.ro...@gmail.com wrote:
 ---
  src/bootchart/store.c | 5 +++--
  1 file changed, 3 insertions(+), 2 deletions(-)

 diff --git a/src/bootchart/store.c b/src/bootchart/store.c
 index cedcba8..2d2ea42 100644
 --- a/src/bootchart/store.c
 +++ b/src/bootchart/store.c
 @@ -34,6 +34,7 @@
  #include time.h

  #include util.h
 +#include time-util.h
  #include strxcpyx.h
  #include store.h
  #include bootchart.h
 @@ -54,14 +55,14 @@ double gettime_ns(void) {

  clock_gettime(CLOCK_MONOTONIC, n);

 -return (n.tv_sec + (n.tv_nsec / 10.0));
 +return (n.tv_sec + (n.tv_nsec / (double) NSEC_PER_SEC));
  }

  static double gettime_up(void) {
  struct timespec n;

  clock_gettime(CLOCK_BOOTTIME, n);
 -return (n.tv_sec + (n.tv_nsec / 10.0));
 +return (n.tv_sec + (n.tv_nsec / (double) NSEC_PER_SEC));
  }

  void log_uptime(void) {
 --
 2.0.4

 ___
 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

Re: [systemd-devel] compile with clang broken

[Thomas H.P. Andersen]

On Fri, Aug 15, 2014 at 10:55 AM, David Herrmann dh.herrm...@gmail.com wrote:
 Hi

 On Thu, Aug 14, 2014 at 8:07 PM, Lennart Poettering
 lenn...@poettering.net wrote:
 On Fri, 18.07.14 16:02, Thomas H.P. Andersen (pho...@gmail.com) wrote:

 1716f6dcf54d4c181c2e2558e3d5414f54c8d9ca (resolved: add LLMNR support
 for looking up names) broke the build on clang.

 src/resolve/resolved-manager.c:553:43: error: non-const static data
 member must be initialized out of line
 uint8_t buffer[CMSG_SPACE(MAX(sizeof(struct in_pktinfo), sizeof(struct
 in6_pktinfo)))

 Moving the MAX(...) to a separate line fixes that problem but another
 error then happens:

 src/resolve/resolved-manager.c:554:25: error: fields must have a
 constant size: 'variable length array in structure' extension will
 never be supported
 uint8_t buffer[CMSG_SPACE(size)

 We have encountered the same problem before and Lennart was able to
 write the code in a different way. Would this be possible here too?

 My sugegstion here would be to maybe rewrite the MAX() macro to use
 __builtin_constant_p() so that it becomes constant if the params are
 constant, and only uses code block when it isn't. Or so...

 http://lists.freedesktop.org/archives/systemd-devel/2014-August/021912.html

 Hm, I don't know whether that works. See the description here:
 https://gcc.gnu.org/onlinedocs/gcc-4.3.3/gcc/Other-Builtins.html

 What you propose is something like my attached patch, I guess? Along
 the lines of:
 (__builtin_constant_p(A)  __builtin_constant_p(B)) ?
 ((A)  (B) ? (A) : (B)) :
 ({ OLD_MAX })

 Thing is, the ELSE case is not considered a compile-time constant by
 LLVM. Therefore, the whole expression is not considered a compile-time
 constant. I don't know whether conditions with __builtin_constant_p()
 are evaluated at the parser-step. The GCC example replaces the ELSE
 case with -1, effectively making both compile-time constants.

 I also added a test-case to make sure __builtin_constant_p doesn't
 evaluate the arguments.

 Can someone with LLVM set up give this a spin?

I still get:
src/resolve/resolved-manager.c:844:43: error: non-const static data
member must be initialized out of line
uint8_t buffer[CMSG_SPACE(MAX(sizeof(struct
in_pktinfo), sizeof(struct in6_pktinfo)))
  ^
I tried moving that to a separate line but then I also still get:
src/resolve/resolved-manager.c:845:25: error: fields must have a
constant size: 'variable length array in structure' extension will
never be supported
uint8_t buffer[CMSG_SPACE(size)
^

I got the following to compile but I have not have time to test it at
all. Too ugly to live I guess...

diff --git a/src/resolve/resolved-dns-stream.c
b/src/resolve/resolved-dns-stream.c
index eb78587..1b415ae 100644
--- a/src/resolve/resolved-dns-stream.c
+++ b/src/resolve/resolved-dns-stream.c
@@ -62,10 +62,14 @@ static int dns_stream_complete(DnsStream *s, int error) {
 }

 static int dns_stream_identify(DnsStream *s) {
+const size_t size = __builtin_constant_p(
+MAX(sizeof(struct in_pktinfo), sizeof(struct in6_pktinfo))) ?
+MAX(sizeof(struct in_pktinfo), sizeof(struct in6_pktinfo)) : 0;
 union {
 struct cmsghdr header; /* For alignment */
-uint8_t buffer[CMSG_SPACE(MAX(sizeof(struct
in_pktinfo), sizeof(struct in6_pktinfo)))
+uint8_t buffer[CMSG_SPACE(size)
+ EXTRA_CMSG_SPACE /* kernel appears
to require extra space */];
+
 } control;
 struct msghdr mh = {};
 struct cmsghdr *cmsg;
@@ -73,6 +77,7 @@ static int dns_stream_identify(DnsStream *s) {
 int r;

 assert(s);
+assert(size  0);

 if (s-identified)
 return 0;
diff --git a/src/resolve/resolved-manager.c b/src/resolve/resolved-manager.c
index bfbdc7d..1342fb1 100644
--- a/src/resolve/resolved-manager.c
+++ b/src/resolve/resolved-manager.c
@@ -839,9 +839,12 @@ fail:

 int manager_recv(Manager *m, int fd, DnsProtocol protocol, DnsPacket **ret) {
 _cleanup_(dns_packet_unrefp) DnsPacket *p = NULL;
+const size_t size = __builtin_constant_p(
+MAX(sizeof(struct in_pktinfo), sizeof(struct in6_pktinfo))) ?
+MAX(sizeof(struct in_pktinfo), sizeof(struct in6_pktinfo)) : 0;
 union {
 struct cmsghdr header; /* For alignment */
-uint8_t buffer[CMSG_SPACE(MAX(sizeof(struct
in_pktinfo), sizeof(struct in6_pktinfo)))
+uint8_t buffer[CMSG_SPACE(size)
+ CMSG_SPACE(int) /* ttl/hoplimit */
+ EXTRA_CMSG_SPACE /* kernel appears
to require extra buffer space */];
 } control;
@@ -855,6 +858,7 @@ int manager_recv(Manager *m, int fd, DnsProtocol
protocol, DnsPacket **ret) {
 assert(m);
 assert(fd

Re: [systemd-devel] compile with clang broken

[Thomas H.P. Andersen]

On Fri, Aug 15, 2014 at 12:35 PM, David Herrmann dh.herrm...@gmail.com wrote:
 Hi

 On Fri, Aug 15, 2014 at 12:29 PM, Thomas H.P. Andersen pho...@gmail.com 
 wrote:
 On Fri, Aug 15, 2014 at 11:49 AM, David Herrmann dh.herrm...@gmail.com 
 wrote:
 Thanks for trying!

 Result is as I expected. Evaluation takes place _after_ validating
 compile-time constants, and thus __builtin_constant_p in combination
 with ?: will not work if not both cases are constant. Maybe it works
 with __builtin_choose_expr()?

 Can you try the attached patch?

 This patch works. It also needs the change to do the calculation to a
 seperate line. Also only if size is const, like so:
 const size_t size = MAX(sizeof(struct in_pktinfo), sizeof(struct 
 in6_pktinfo));

 Again, thanks for trying it out!

no problem. I have inserted the relevant error messages for the two
non-working cases.

 I don't understand your comment, though. You're saying this works:

 const size_t size = MAX(...);
 uint8_t buffer[CMSG_SPACE(size) +...];

 ...but this doesn't work:

 uint8_t buffer[CMSG_SPACE(MAX(...)) +...];

src/resolve/resolved-dns-stream.c:67:43: error: non-const static data
member must be initialized out of line
uint8_t buffer[CMSG_SPACE(MAX(sizeof(struct
in_pktinfo), sizeof(struct in6_pktinfo)))
  ^

 ...and this doesn't work either (mind the dropped 'const'):

 size_t size = MAX(...);
 uint8_t buffer[CMSG_SPACE(size) +...];

src/resolve/resolved-dns-stream.c:68:25: error: fields must have a
constant size: 'variable length array in structure' extension will
never be supported
uint8_t buffer[CMSG_SPACE(size)
^

 Hm. This is weird. Maybe CMSG_SPACE does something weird. I'll see.
 David
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [PATCH] resolved: fix warnings

[Thomas H.P. Andersen]

On Sat, Jul 19, 2014 at 10:37 AM, Thomas H.P. Andersen pho...@gmail.com wrote:
 From: Thomas Hindoe Paaboel Andersen pho...@gmail.com

 ---
  src/resolve/resolved-dns-scope.c | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)

 diff --git a/src/resolve/resolved-dns-scope.c 
 b/src/resolve/resolved-dns-scope.c
 index 190c5f4..41514a5 100644
 --- a/src/resolve/resolved-dns-scope.c
 +++ b/src/resolve/resolved-dns-scope.c
 @@ -292,7 +292,7 @@ int dns_scope_llmnr_membership(DnsScope *s, bool b) {
  if (s-family == AF_INET) {
  struct ip_mreqn mreqn = {
  .imr_multiaddr = LLMNR_MULTICAST_IPV4_ADDRESS,
 -.imr_ifindex = s-link-ifindex,
 +.imr_ifindex = s-link ? s-link-ifindex : 0,
  };

  fd = manager_llmnr_ipv4_udp_fd(s-manager);
 @@ -305,7 +305,7 @@ int dns_scope_llmnr_membership(DnsScope *s, bool b) {
  } else if (s-family == AF_INET6) {
  struct ipv6_mreq mreq = {
  .ipv6mr_multiaddr = LLMNR_MULTICAST_IPV6_ADDRESS,
 -.ipv6mr_interface = s-link-ifindex,
 +.ipv6mr_interface = s-link ? s-link-ifindex : 0,
  };

  fd = manager_llmnr_ipv6_udp_fd(s-manager);
 --
 1.9.3


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

[systemd-devel] [PATCH] resolved: avoid possible dereference of null pointer

[Thomas H.P. Andersen]

From: Thomas Hindoe Paaboel Andersen pho...@gmail.com

In dns_scope_make_reply_packet the structs q, answer, and soa can be
null. We should check for null before reading their fields.
---
 src/resolve/resolved-dns-scope.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/src/resolve/resolved-dns-scope.c b/src/resolve/resolved-dns-scope.c
index 9567056..3c2f6e6 100644
--- a/src/resolve/resolved-dns-scope.c
+++ b/src/resolve/resolved-dns-scope.c
@@ -412,7 +412,9 @@ static int dns_scope_make_reply_packet(
 
 assert(s);
 
-if (q-n_keys = 0  answer-n_rrs = 0  soa-n_rrs = 0)
+if ((!q || q-n_keys = 0)
+ (!answer || answer-n_rrs = 0)
+ (!soa || soa-n_rrs = 0))
 return -EINVAL;
 
 r = dns_packet_new(p, s-protocol, 0);
-- 
1.9.3

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

Re: [systemd-devel] [PATCH] resolved: avoid possible dereference of null pointer

[Thomas H.P. Andersen]

On Sun, Aug 3, 2014 at 10:56 PM, Zbigniew Jędrzejewski-Szmek
zbys...@in.waw.pl wrote:
 On Sun, Aug 03, 2014 at 10:53:05PM +0200, Thomas H.P. Andersen wrote:
 From: Thomas Hindoe Paaboel Andersen pho...@gmail.com

 In dns_scope_make_reply_packet the structs q, answer, and soa can be
 null. We should check for null before reading their fields.
 Looks OK.
Thanks. I was in doubt about the style. Pushed it now.
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] Warnings from recent commits

[Thomas H.P. Andersen]

On Mon, Jul 21, 2014 at 4:18 AM, Zbigniew Jędrzejewski-Szmek
zbys...@in.waw.pl wrote:
 On Thu, Jul 17, 2014 at 08:51:52PM +0200, Thomas H.P. Andersen wrote:
 From recent commits I have noticed the following new issues from
 static analysis with scan-build and with clang. I am not sure how they
 should be fixed (or even if) but I just though I would let you know.

 1) src/shared/barrier.c in barrier_read starting at line 274

 if (pfd[1].revents) {
 len = read(b-them, buf, sizeof(buf));
 ...
 } else if (pfd[0].revents  (POLLHUP | POLLERR | POLLNVAL)) {
 ...
 buf = BARRIER_ABORTION;
 }

 If neither if/else if are true then buf will be used unset.

 2) src/resolve/resolved-dns-scope.c in dns_scope_tcp_socket
 if s-link is null then ifindex will not be set but will be used later in:

 } else if (srv-family == AF_INET6) {
 sa.in6.sin6_port = htobe16(53);
 sa.in6.sin6_addr = srv-address.in6;
 sa.in6.sin6_scope_id = ifindex;
 salen = sizeof(sa.in6);

 3) I see a couple of these:

 In file included from src/resolve/resolved-gperf.c:8:
 In file included from ./src/resolve/resolved.h:34:
 In file included from ./src/resolve/resolved-dns-query.h:33:
 In file included from ./src/resolve/resolved-dns-scope.h:33:
 ./src/resolve/resolved-dns-cache.h:45:3: warning: redefinition of
 typedef 'DnsCacheItem' is a C11 feature [-Wtypedef-redefinition]
 } DnsCacheItem;
   ^
 ./src/resolve/resolved-dns-cache.h:31:29: note: previous definition is here
 typedef struct DnsCacheItem DnsCacheItem;

 I'd simply add -Wno-typedef-redefinition to CLAGS... This is one
 of those cases where the medicine is worse than the disease.

Fine by me. Typedef redefinition was added in gcc 4.6 so we will need that.
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] Warnings from recent commits

[Thomas H.P. Andersen]

On Thu, Jul 17, 2014 at 8:51 PM, Thomas H.P. Andersen pho...@gmail.com wrote:
 From recent commits I have noticed the following new issues from
 static analysis with scan-build and with clang. I am not sure how they
 should be fixed (or even if) but I just though I would let you know.

 1) src/shared/barrier.c in barrier_read starting at line 274

 if (pfd[1].revents) {
 len = read(b-them, buf, sizeof(buf));
 ...
 } else if (pfd[0].revents  (POLLHUP | POLLERR | POLLNVAL)) {
 ...
 buf = BARRIER_ABORTION;
 }

 If neither if/else if are true then buf will be used unset.

 2) src/resolve/resolved-dns-scope.c in dns_scope_tcp_socket
 if s-link is null then ifindex will not be set but will be used later in:

 } else if (srv-family == AF_INET6) {
 sa.in6.sin6_port = htobe16(53);
 sa.in6.sin6_addr = srv-address.in6;
 sa.in6.sin6_scope_id = ifindex;
 salen = sizeof(sa.in6);

Zbigniew dealt with 2) in 901fd8164797f3eeb9921c85915dc409d49ab5d8.

 3) I see a couple of these:

 In file included from src/resolve/resolved-gperf.c:8:
 In file included from ./src/resolve/resolved.h:34:
 In file included from ./src/resolve/resolved-dns-query.h:33:
 In file included from ./src/resolve/resolved-dns-scope.h:33:
 ./src/resolve/resolved-dns-cache.h:45:3: warning: redefinition of
 typedef 'DnsCacheItem' is a C11 feature [-Wtypedef-redefinition]
 } DnsCacheItem;
   ^
 ./src/resolve/resolved-dns-cache.h:31:29: note: previous definition is here
 typedef struct DnsCacheItem DnsCacheItem;
 ^
 1 warning generated.
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

[systemd-devel] [PATCH] resolved: fix warnings

[Thomas H.P. Andersen]

From: Thomas Hindoe Paaboel Andersen pho...@gmail.com

---
 src/resolve/resolved-dns-scope.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/resolve/resolved-dns-scope.c b/src/resolve/resolved-dns-scope.c
index 190c5f4..41514a5 100644
--- a/src/resolve/resolved-dns-scope.c
+++ b/src/resolve/resolved-dns-scope.c
@@ -292,7 +292,7 @@ int dns_scope_llmnr_membership(DnsScope *s, bool b) {
 if (s-family == AF_INET) {
 struct ip_mreqn mreqn = {
 .imr_multiaddr = LLMNR_MULTICAST_IPV4_ADDRESS,
-.imr_ifindex = s-link-ifindex,
+.imr_ifindex = s-link ? s-link-ifindex : 0,
 };
 
 fd = manager_llmnr_ipv4_udp_fd(s-manager);
@@ -305,7 +305,7 @@ int dns_scope_llmnr_membership(DnsScope *s, bool b) {
 } else if (s-family == AF_INET6) {
 struct ipv6_mreq mreq = {
 .ipv6mr_multiaddr = LLMNR_MULTICAST_IPV6_ADDRESS,
-.ipv6mr_interface = s-link-ifindex,
+.ipv6mr_interface = s-link ? s-link-ifindex : 0,
 };
 
 fd = manager_llmnr_ipv6_udp_fd(s-manager);
-- 
1.9.3

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

[systemd-devel] Warnings from recent commits

[Thomas H.P. Andersen]

From recent commits I have noticed the following new issues from
static analysis with scan-build and with clang. I am not sure how they
should be fixed (or even if) but I just though I would let you know.

1) src/shared/barrier.c in barrier_read starting at line 274

if (pfd[1].revents) {
len = read(b-them, buf, sizeof(buf));
...
} else if (pfd[0].revents  (POLLHUP | POLLERR | POLLNVAL)) {
...
buf = BARRIER_ABORTION;
}

If neither if/else if are true then buf will be used unset.

2) src/resolve/resolved-dns-scope.c in dns_scope_tcp_socket
if s-link is null then ifindex will not be set but will be used later in:

} else if (srv-family == AF_INET6) {
sa.in6.sin6_port = htobe16(53);
sa.in6.sin6_addr = srv-address.in6;
sa.in6.sin6_scope_id = ifindex;
salen = sizeof(sa.in6);

3) I see a couple of these:

In file included from src/resolve/resolved-gperf.c:8:
In file included from ./src/resolve/resolved.h:34:
In file included from ./src/resolve/resolved-dns-query.h:33:
In file included from ./src/resolve/resolved-dns-scope.h:33:
./src/resolve/resolved-dns-cache.h:45:3: warning: redefinition of
typedef 'DnsCacheItem' is a C11 feature [-Wtypedef-redefinition]
} DnsCacheItem;
  ^
./src/resolve/resolved-dns-cache.h:31:29: note: previous definition is here
typedef struct DnsCacheItem DnsCacheItem;
^
1 warning generated.
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [PATCH] sysv-generator: do not generate 'Wants' symlinks to generated service files that will be shadowed by a native unit.

[Thomas H.P. Andersen]

On Wed, Jul 16, 2014 at 11:57 AM, Jon Severinsson j...@severinsson.net wrote:
 ---
  src/sysv-generator/sysv-generator.c | 29 +++--
  1 file changed, 27 insertions(+), 2 deletions(-)

 diff --git a/src/sysv-generator/sysv-generator.c 
 b/src/sysv-generator/sysv-generator.c
 index 5206279..aff5fd6 100644
 --- a/src/sysv-generator/sysv-generator.c
 +++ b/src/sysv-generator/sysv-generator.c
 @@ -113,7 +113,26 @@ static int add_symlink(const char *service, const char 
 *where) {
  return 1;
  }

 -static int generate_unit_file(SysvStub *s) {
 +static int native_unit_exists(LookupPaths lp, char *name) {
 +char **p;
 +
 +STRV_FOREACH(p, lp.unit_path) {
 +struct stat st;
 +_cleanup_free_ char *path = NULL;
 +
 +path = strjoin(*p, /, name, NULL);
 +if (!path)
 +return -ENOMEM;
 +
 +if (lstat(path, st)  0)
 +continue;
 +
 +return 1;
 +}
 +return 0;
 +}
 +
 +static int generate_unit_file(LookupPaths lp, SysvStub *s) {
  char *unit;
  char **p;
  _cleanup_fclose_ FILE *f = NULL;
 @@ -190,6 +209,12 @@ static int generate_unit_file(SysvStub *s) {
  if (s-reload)
  fprintf(f, ExecReload=%s reload\n, s-path);

 +/* Do not generate 'Wants' symlinks to the generated service file if 
 it
 + * will be shadowed by an existing native unit, as the symlinks would
 + * not be shadowed but would pull the native unit instead. */
 +if (native_unit_exists(lp, s-name))
 +return 0;
 +
Any reason that we should not just put the native_unit_exists check in
enumerate_sysv instead and skip generating a unit at all?

  STRV_FOREACH(p, s-wanted_by) {
  r = add_symlink(s-name, *p);
  if (r  0)
 @@ -918,7 +943,7 @@ int main(int argc, char *argv[]) {
  if (q  0)
  continue;

 -q = generate_unit_file(service);
 +q = generate_unit_file(lp, service);
  if (q  0)
  continue;
  }
 --
 2.0.1

 ___
 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

[systemd-devel] [PATCH] sd-dhcp6-client: check return value

[Thomas H.P. Andersen]

From: Thomas Hindoe Paaboel Andersen pho...@gmail.com

Checking the return values seems to have been forgotten in
ed6ee21953dac9c78383da00bc4514ece6b75ab5
---
 src/libsystemd-network/sd-dhcp6-client.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/src/libsystemd-network/sd-dhcp6-client.c 
b/src/libsystemd-network/sd-dhcp6-client.c
index 6e00662..1d6e42e 100644
--- a/src/libsystemd-network/sd-dhcp6-client.c
+++ b/src/libsystemd-network/sd-dhcp6-client.c
@@ -254,6 +254,8 @@ static int client_send_message(sd_dhcp6_client *client) {
 
 r = dhcp6_option_append(opt, optlen,
 DHCP6_OPTION_RAPID_COMMIT, 0, NULL);
+if (r  0)
+return r;
 
 r = dhcp6_option_append_ia(opt, optlen, client-ia_na);
 if (r  0)
-- 
1.9.3

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

[systemd-devel] [PATCH] coredump: vacuum - fix calculation of 10% of fs size for MaxUse

[Thomas H.P. Andersen]

From: Thomas Hindoe Paaboel Andersen pho...@gmail.com

---
 src/journal/coredump-vacuum.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/journal/coredump-vacuum.c b/src/journal/coredump-vacuum.c
index ad2e2fa..125bb3a 100644
--- a/src/journal/coredump-vacuum.c
+++ b/src/journal/coredump-vacuum.c
@@ -104,8 +104,8 @@ static bool vacuum_necessary(int fd, off_t sum, off_t 
keep_free, off_t max_use)
 if (max_use  DEFAULT_MAX_USE_LOWER)
 max_use = DEFAULT_MAX_USE_LOWER;
 }
-
-max_use = DEFAULT_MAX_USE_LOWER;
+else
+max_use = DEFAULT_MAX_USE_LOWER;
 } else
 max_use = PAGE_ALIGN(max_use);
 
-- 
1.9.3

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

Re: [systemd-devel] [PATCH v4 1/2] Move handling of sysv initscripts to a generator

[Thomas H.P. Andersen]

On Tue, Jun 24, 2014 at 7:56 PM, Alexey Shabalin a.shaba...@gmail.com wrote:
 2014-06-07 3:01 GMT+04:00 Thomas H.P. Andersen pho...@gmail.com:
 On Fri, Jun 6, 2014 at 4:09 PM, Lennart Poettering
 lenn...@poettering.net wrote:
 On Sat, 31.05.14 23:29, Thomas H.P. Andersen (pho...@gmail.com) wrote:

 +
 +if (s-sysv_start_priority  0)
 +fprintf(f, SysVStartPriority=%d\n, 
 s-sysv_start_priority);
 +
 + if (s-pid_file)
 + fprintf(f, PidFile=%s\n, s-pid_file);

 rename PidFile to PIDFile, please.

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

[systemd-devel] [PATCH] consistently order cleanup attribute before type

[Thomas H.P. Andersen]

From: Thomas Hindoe Paaboel Andersen pho...@gmail.com

---
 src/cgls/cgls.c| 2 +-
 src/journal/journal-remote-parse.c | 4 ++--
 src/journal/journal-remote.c   | 8 
 src/journal/microhttpd-util.c  | 2 +-
 src/nspawn/nspawn.c| 2 +-
 src/readahead/readahead-common.c   | 2 +-
 src/shared/fileio.c| 2 +-
 src/shared/sleep-config.c  | 2 +-
 src/shared/socket-label.c  | 2 +-
 src/test/test-path-util.c  | 2 +-
 10 files changed, 14 insertions(+), 14 deletions(-)

diff --git a/src/cgls/cgls.c b/src/cgls/cgls.c
index bec9b59..052ac8f 100644
--- a/src/cgls/cgls.c
+++ b/src/cgls/cgls.c
@@ -130,7 +130,7 @@ static int parse_argv(int argc, char *argv[]) {
 int main(int argc, char *argv[]) {
 int r = 0, retval = EXIT_FAILURE;
 int output_flags;
-char _cleanup_free_ *root = NULL;
+_cleanup_free_ char *root = NULL;
 _cleanup_bus_unref_ sd_bus *bus = NULL;
 
 log_parse_environment();
diff --git a/src/journal/journal-remote-parse.c 
b/src/journal/journal-remote-parse.c
index 239ff38..dbdf02a 100644
--- a/src/journal/journal-remote-parse.c
+++ b/src/journal/journal-remote-parse.c
@@ -177,7 +177,7 @@ static int fill_fixed_size(RemoteSource *source, void 
**data, size_t size) {
 
 static int get_data_size(RemoteSource *source) {
 int r;
-void _cleanup_free_ *data = NULL;
+_cleanup_free_ void *data = NULL;
 
 assert(source);
 assert(source-state == STATE_DATA_START);
@@ -215,7 +215,7 @@ static int get_data_data(RemoteSource *source, void **data) 
{
 
 static int get_data_newline(RemoteSource *source) {
 int r;
-char _cleanup_free_ *data = NULL;
+_cleanup_free_ char *data = NULL;
 
 assert(source);
 assert(source-state == STATE_DATA_FINISH);
diff --git a/src/journal/journal-remote.c b/src/journal/journal-remote.c
index 31401fb..a31dc2c 100644
--- a/src/journal/journal-remote.c
+++ b/src/journal/journal-remote.c
@@ -139,7 +139,7 @@ static int spawn_curl(char* url) {
 
 static int spawn_getter(char *getter, char *url) {
 int r;
-char _cleanup_strv_free_ **words = NULL;
+_cleanup_strv_free_ char **words = NULL;
 
 assert(getter);
 words = strv_split_quoted(getter);
@@ -154,7 +154,7 @@ static int spawn_getter(char *getter, char *url) {
 }
 
 static int open_output(Writer *s, const char* url) {
-char _cleanup_free_ *name, *output = NULL;
+_cleanup_free_ char *name, *output = NULL;
 char *c;
 int r;
 
@@ -745,8 +745,8 @@ static int remoteserver_init(RemoteServer *s) {
 }
 
 if (arg_url) {
-char _cleanup_free_ *url = NULL;
-char _cleanup_strv_free_ **urlv = strv_new(arg_url, 
/entries, NULL);
+_cleanup_free_ char *url = NULL;
+_cleanup_strv_free_ char **urlv = strv_new(arg_url, 
/entries, NULL);
 if (!urlv)
 return log_oom();
 url = strv_join(urlv, );
diff --git a/src/journal/microhttpd-util.c b/src/journal/microhttpd-util.c
index 9a8d5c6..007cb5d 100644
--- a/src/journal/microhttpd-util.c
+++ b/src/journal/microhttpd-util.c
@@ -217,7 +217,7 @@ int check_permissions(struct MHD_Connection *connection, 
int *code) {
 const union MHD_ConnectionInfo *ci;
 gnutls_session_t session;
 gnutls_x509_crt_t client_cert;
-char _cleanup_free_ *buf = NULL;
+_cleanup_free_ char *buf = NULL;
 int r;
 
 assert(connection);
diff --git a/src/nspawn/nspawn.c b/src/nspawn/nspawn.c
index 8270348..212f722 100644
--- a/src/nspawn/nspawn.c
+++ b/src/nspawn/nspawn.c
@@ -824,7 +824,7 @@ static int setup_timezone(const char *dest) {
 }
 
 static int setup_resolv_conf(const char *dest) {
-char _cleanup_free_ *where = NULL;
+_cleanup_free_ char *where = NULL;
 
 assert(dest);
 
diff --git a/src/readahead/readahead-common.c b/src/readahead/readahead-common.c
index 890886e..eda99e8 100644
--- a/src/readahead/readahead-common.c
+++ b/src/readahead/readahead-common.c
@@ -226,7 +226,7 @@ int open_inotify(void) {
 }
 
 ReadaheadShared *shared_get(void) {
-int _cleanup_close_ fd = -1;
+_cleanup_close_ int fd = -1;
 ReadaheadShared *m = NULL;
 
 mkdirs();
diff --git a/src/shared/fileio.c b/src/shared/fileio.c
index c7b2cd8..c580624 100644
--- a/src/shared/fileio.c
+++ b/src/shared/fileio.c
@@ -708,7 +708,7 @@ int write_env_file(const char *fname, char **l) {
 
 int executable_is_script(const char *path, char **interpreter) {
 int r;
-char _cleanup_free_ *line = NULL;
+_cleanup_free_ char *line = NULL;
 int len;
 char *ans;
 
diff --git a/src/shared/sleep-config.c b/src/shared/sleep-config.c
index 1972cdb..867e4ed 100644
--- a/src/shared/sleep-config.c
+++ b/src/shared/sleep-config.c
@@ -49,7 +49,7 @@ int 

[systemd-devel] [PATCH] tmpfiles: copy - stop if chown/chmod fails

[Thomas H.P. Andersen]

From: Thomas Hindoe Paaboel Andersen pho...@gmail.com

after 19f3934057d20c63f4c95791312038a41b4666d0 the errors from chown/chmod
will be cleared before being read. Presumably to clear the possible EEXIST
from copying to an existing directory.
Note: with this patch we stop immediately on error in chown/chmod while before
we would attempt to copy anyway.
---
 src/shared/copy.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/shared/copy.c b/src/shared/copy.c
index 867e49b..5434e7b 100644
--- a/src/shared/copy.c
+++ b/src/shared/copy.c
@@ -181,10 +181,10 @@ static int fd_copy_directory(int df, const char *from, 
const struct stat *st, in
 
 if (created) {
 if (fchown(fdt, st-st_uid, st-st_gid)  0)
-r = -errno;
+return -errno;
 
 if (fchmod(fdt, st-st_mode  0)  0)
-r = -errno;
+return -errno;
 }
 
 r = 0;
-- 
1.9.3

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

Re: [systemd-devel] [PATCH] dd-dhcp-server: fix a leak

[Thomas H.P. Andersen]

On Fri, Jun 13, 2014 at 10:58 PM, Thomas H.P. Andersen pho...@gmail.com wrote:
 From: Thomas Hindoe Paaboel Andersen pho...@gmail.com

 We must use free instead of dhcp_lease_free here to avoid freeing
 client_id.data.
 ---
  src/libsystemd-network/sd-dhcp-server.c | 4 +++-
  1 file changed, 3 insertions(+), 1 deletion(-)

 diff --git a/src/libsystemd-network/sd-dhcp-server.c 
 b/src/libsystemd-network/sd-dhcp-server.c
 index 17c19cc..38f93c1 100644
 --- a/src/libsystemd-network/sd-dhcp-server.c
 +++ b/src/libsystemd-network/sd-dhcp-server.c
 @@ -694,8 +694,10 @@ int dhcp_server_handle_message(sd_dhcp_server *server, 
 DHCPMessage *message,
  lease-address = req-requested_ip;
  lease-client_id.data = 
 memdup(req-client_id.data,
 
 req-client_id.length);
 -if (!lease-client_id.data)
 +if (!lease-client_id.data) {
 +free(lease);
  return -ENOMEM;
 +}
  lease-client_id.length = 
 req-client_id.length;
  } else
  lease = existing_lease;
 --
 1.9.3


I ended up committing this one directly with a some other patches.
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

[systemd-devel] [PATCH] sd-dhcp-server: remove unused cleanup function

[Thomas H.P. Andersen]

From: Thomas Hindoe Paaboel Andersen pho...@gmail.com

Removes _cleanup_dhcp_lease_free_. While the automatic cleanup
functions are great to have this one is never used and causes
a warning in clang.
---
 src/libsystemd-network/sd-dhcp-server.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/src/libsystemd-network/sd-dhcp-server.c 
b/src/libsystemd-network/sd-dhcp-server.c
index 4ce1054..17c19cc 100644
--- a/src/libsystemd-network/sd-dhcp-server.c
+++ b/src/libsystemd-network/sd-dhcp-server.c
@@ -105,9 +105,6 @@ static void dhcp_lease_free(DHCPLease *lease) {
 free(lease);
 }
 
-DEFINE_TRIVIAL_CLEANUP_FUNC(DHCPLease*, dhcp_lease_free);
-#define _cleanup_dhcp_lease_free_ _cleanup_(dhcp_lease_freep)
-
 sd_dhcp_server *sd_dhcp_server_unref(sd_dhcp_server *server) {
 if (server  REFCNT_DEC(server-n_ref) = 0) {
 DHCPLease *lease;
-- 
1.9.3

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

[systemd-devel] [PATCH] networkd: link - check returned value from set_lease_pool

[Thomas H.P. Andersen]

From: Thomas Hindoe Paaboel Andersen pho...@gmail.com

---
 src/network/networkd-link.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/src/network/networkd-link.c b/src/network/networkd-link.c
index 81872f7..baa0756 100644
--- a/src/network/networkd-link.c
+++ b/src/network/networkd-link.c
@@ -1723,6 +1723,8 @@ static int link_configure(Link *link) {
 pool_start.s_addr = 
htobe32(be32toh(address-in_addr.in.s_addr) + 1);
 r = sd_dhcp_server_set_lease_pool(link-dhcp_server,
   pool_start, 32);
+if (r  0)
+return r;
 
 break;
 }
-- 
1.9.3

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

[systemd-devel] [PATCH] dd-dhcp-server: fix a leak

[Thomas H.P. Andersen]

From: Thomas Hindoe Paaboel Andersen pho...@gmail.com

We must use free instead of dhcp_lease_free here to avoid freeing
client_id.data.
---
 src/libsystemd-network/sd-dhcp-server.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/src/libsystemd-network/sd-dhcp-server.c 
b/src/libsystemd-network/sd-dhcp-server.c
index 17c19cc..38f93c1 100644
--- a/src/libsystemd-network/sd-dhcp-server.c
+++ b/src/libsystemd-network/sd-dhcp-server.c
@@ -694,8 +694,10 @@ int dhcp_server_handle_message(sd_dhcp_server *server, 
DHCPMessage *message,
 lease-address = req-requested_ip;
 lease-client_id.data = 
memdup(req-client_id.data,

req-client_id.length);
-if (!lease-client_id.data)
+if (!lease-client_id.data) {
+free(lease);
 return -ENOMEM;
+}
 lease-client_id.length = 
req-client_id.length;
 } else
 lease = existing_lease;
-- 
1.9.3

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

Re: [systemd-devel] [PATCH] Fix spelling mistake, scirpt -- script

[Thomas H.P. Andersen]

On Thu, Jun 12, 2014 at 5:41 PM, Colin King colin.k...@canonical.com wrote:
 From: Colin Ian King colin.k...@canonical.com

 Signed-off-by: Colin Ian King colin.k...@canonical.com
 ---
  src/sysv-generator/sysv-generator.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

 diff --git a/src/sysv-generator/sysv-generator.c 
 b/src/sysv-generator/sysv-generator.c
 index 0b8d8f7..18dae5c 100644
 --- a/src/sysv-generator/sysv-generator.c
 +++ b/src/sysv-generator/sysv-generator.c
 @@ -791,7 +791,7 @@ static int set_dependencies_from_rcnd(LookupPaths lp, 
 Hashmap *all_services) {
  if (hashmap_contains(all_services, name))
  service = hashmap_get(all_services, 
 name);
  else {
 -log_warning(Could not find init 
 scirpt for %s, name);
 +log_warning(Could not find init 
 script for %s, name);
  continue;
  }

 --
 2.0.0

Oops. Applied, but without the signed-of-by. We don't use that. Thanks.
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

[systemd-devel] [PATCH] networkd: link - avoid null pointer deref

[Thomas H.P. Andersen]

From: Thomas Hindoe Paaboel Andersen pho...@gmail.com

---
 src/network/networkd-link.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/network/networkd-link.c b/src/network/networkd-link.c
index baa0756..39c8329 100644
--- a/src/network/networkd-link.c
+++ b/src/network/networkd-link.c
@@ -2161,8 +2161,8 @@ int link_save(Link *link) {
 DHCP_USE_DNS=%s\n
 DHCP_USE_NTP=%s\n,
 link-lease_file,
-yes_no(link-network-dhcp_dns),
-yes_no(link-network-dhcp_ntp));
+yes_no(link-network  link-network-dhcp_dns),
+yes_no(link-network  link-network-dhcp_ntp));
 } else
 unlink(link-lease_file);
 
-- 
1.9.3

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

Re: [systemd-devel] [PATCH 0/4] looking at _cleanup_free_ usage

[Thomas H.P. Andersen]

On Fri, Jun 13, 2014 at 7:04 PM, Tom Gundersen t...@jklm.no wrote:
 On Fri, Jun 13, 2014 at 6:48 PM, Andreas Henriksson andr...@fatal.se wrote:
 Hello all!

 Hi Andreas,

 I was recently bitten by a problem which I found the solution to in
 http://lists.freedesktop.org/archives/systemd-commits/2013-October/004421.html

 The second patch there made me curious to look at more occurances.

 Thanks for looking at this!

 I do not have prior experience with _cleanup_free_ but as I understand
 it in general the pointer gets free()d when it goes out of scope
 and thus must be initialized before that happens to avoid memory corruption.

 Correct.

 This thread contains patches for the occurences I found which seems
 to violate that.
 Please review them carefully!!!
 (I have not tried to trigger the actual codepaths.)

 We should (IMHO) always initialize the _cleanup_ variables when
 declaring them, even if it so happens that they would anyway later get
 initialized. Introducing bugs later on is too easy other wise.

 With that in mind, your patches all look good to me. So applied them
 all. Thanks! (We don't do s-o-b, so dropped those).

 Feel free to also follow up with general discussion on best practices.

 Should CODING_STYLE also contain a blurb next to the recommendation
 to use _cleanup_free_ that reminds/warns people it must be initialized
 before it goes out of scope?

 Even better, I think; always initialize when declaring.

 Should helper function patterns be changed to set *ret = NULL early
 on to try to prevent cases where it does not initialize a variable
 on error cases?
 ie.

 int myhelper(..., char **ret) {
 int x, y, z;

 assert(...);

 // This is just for extra safety so the value always gets 
 initialized.
 *ret = NULL;

 ...

 if (x != y)
 return -EEXAMPLE;

 ...

 *ret = strdup(example);
 return 0;
 }

 No, I don't think so. We try to never touch the passed *ret (and
 similar) pointers on failure. So you get *ret if and only if the whole
 function succeeds.

 Is the same problem also affecting other _cleanup_whatever_ variants?

 Is there a general solution in the macro itself to make sure
 the variable it is used on always gets initialized?

 Should CODING_STYLE be changed to recommend this:
 _cleanup_free_ char *myptr = strdup(example);
 .. instead of this:
 _cleanup_free_ char *myptr;

 myptr = strdup(example);
 ... to make it easier to spot (or grep for) errors?

 We try to avoid calling functions when declaring variables, so I'd just do:

 _cleanup_free_ char *myptr = NULL;

 myptr = strdup(example);

 Are there any static code analyzers that can catch these errors that can
 be used to avoid relying on manual reviews catching these errors?

 As far as I know, there is work to make the llvm static analyzer work
 with this, but at the moment that is not ready.

Compiling with clang helps to spot some of these issues but the static
analyzer could do it better. There is a llvm bug to make scan-build
understand the cleanup attribute. That should both make it warn for
this and get rid of 200+ false positives for memleaks and dead
assignments. It would make scan-build a much more useful tool for
systemd so if we have friends in llvm then please point them to this
bug:
http://llvm.org/bugs/show_bug.cgi?id=3888
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

[systemd-devel] [PATCH] cryptsetup: check that password is not null

[Thomas H.P. Andersen]

From: Thomas Hindoe Paaboel Andersen pho...@gmail.com

Beef up the assert to protect against passing null to strlen.

Found with scan-build.
---
 src/cryptsetup/cryptsetup.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/cryptsetup/cryptsetup.c b/src/cryptsetup/cryptsetup.c
index 812b32f..a67d85e 100644
--- a/src/cryptsetup/cryptsetup.c
+++ b/src/cryptsetup/cryptsetup.c
@@ -344,7 +344,7 @@ static int attach_tcrypt(struct crypt_device *cd,
 
 assert(cd);
 assert(name);
-assert(key_file || passwords);
+assert(key_file || (passwords  passwords[0]));
 
 if (arg_tcrypt_hidden)
 params.flags |= CRYPT_TCRYPT_HIDDEN_HEADER;
-- 
1.9.3

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

Re: [systemd-devel] [PATCH] cryptsetup: check that password is not null

[Thomas H.P. Andersen]

On Thu, Jun 12, 2014 at 11:08 PM, Greg KH gre...@linuxfoundation.org wrote:
 On Thu, Jun 12, 2014 at 10:55:50PM +0200, Thomas H.P. Andersen wrote:
 From: Thomas Hindoe Paaboel Andersen pho...@gmail.com

 Beef up the assert to protect against passing null to strlen.

 Found with scan-build.
 ---
  src/cryptsetup/cryptsetup.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

 diff --git a/src/cryptsetup/cryptsetup.c b/src/cryptsetup/cryptsetup.c
 index 812b32f..a67d85e 100644
 --- a/src/cryptsetup/cryptsetup.c
 +++ b/src/cryptsetup/cryptsetup.c
 @@ -344,7 +344,7 @@ static int attach_tcrypt(struct crypt_device *cd,

  assert(cd);
  assert(name);
 -assert(key_file || passwords);
 +assert(key_file || (passwords  passwords[0]));

 Shouldn't strlen of an empty string just return 0?

Passing null to strlen is undefined behavior and seg faults reliably for me.

 What is this fixing really?

Just a theoretical problem found with static analysis. Not sure if we
can actually hit the problem but the current assert gives a false
sense of security.

 thanks,

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

Re: [systemd-devel] [PATCH] cryptsetup: check that password is not null

[Thomas H.P. Andersen]

On Fri, Jun 13, 2014 at 12:01 AM, Greg KH gre...@linuxfoundation.org wrote:
 On Thu, Jun 12, 2014 at 11:49:35PM +0200, Thomas H.P. Andersen wrote:
 On Thu, Jun 12, 2014 at 11:08 PM, Greg KH gre...@linuxfoundation.org wrote:
  On Thu, Jun 12, 2014 at 10:55:50PM +0200, Thomas H.P. Andersen wrote:
  From: Thomas Hindoe Paaboel Andersen pho...@gmail.com
 
  Beef up the assert to protect against passing null to strlen.
 
  Found with scan-build.
  ---
   src/cryptsetup/cryptsetup.c | 2 +-
   1 file changed, 1 insertion(+), 1 deletion(-)
 
  diff --git a/src/cryptsetup/cryptsetup.c b/src/cryptsetup/cryptsetup.c
  index 812b32f..a67d85e 100644
  --- a/src/cryptsetup/cryptsetup.c
  +++ b/src/cryptsetup/cryptsetup.c
  @@ -344,7 +344,7 @@ static int attach_tcrypt(struct crypt_device *cd,
 
   assert(cd);
   assert(name);
  -assert(key_file || passwords);
  +assert(key_file || (passwords  passwords[0]));
 
  Shouldn't strlen of an empty string just return 0?

 Passing null to strlen is undefined behavior and seg faults reliably for me.

 NULL, yes, but that's already checked.  Passing a string that is \0
 should work properly, which is all you added the test for here, right?

 Oh doh, that's an array of an array, nevermind, the patch is correct,
 my fault.

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

Re: [systemd-devel] [PATCH v4 1/2] Move handling of sysv initscripts to a generator

[Thomas H.P. Andersen]

On Fri, Jun 6, 2014 at 4:09 PM, Lennart Poettering
lenn...@poettering.net wrote:
 On Sat, 31.05.14 23:29, Thomas H.P. Andersen (pho...@gmail.com) wrote:

 Thanks! Awesome work! I am enjoying this!

 A few comments:

 +
 +static int generate_unit_file(SysvStub *s) {
 +char *unit;
 +char **p;
 +_cleanup_fclose_ FILE *f = NULL;
 +_cleanup_free_ char *before = NULL;
 +_cleanup_free_ char *after = NULL;
 +_cleanup_free_ char *conflicts = NULL;
 +int r;
 +
 +before = strv_join(s-before,  );
 +after = strv_join(s-after,  );
 +conflicts = strv_join(s-conflicts,  );

 misses OOM checks!

 +
 +unit = strjoin(arg_dest, /, s-name, NULL);
 +if (!unit)
 +return log_oom();
 +
 +f = fopen(unit, wxe);
 +if (!f) {
 +log_error(Failed to create unit file %s: %m, unit);
 +return -errno;
 +}
 +
 +fprintf(f,
 +# Automatically generated by systemd-sysv-generator\n\n
 +[Unit]\n
 +SourcePath=%s\n
 +Description=%s\n,
 +s-path, s-description);
 +
 +if (!isempty(before))
 +fprintf(f, Before=%s\n, before);
 +if (!isempty(after))
 +fprintf(f, After=%s\n, after);
 +if (!isempty(conflicts))
 +fprintf(f, Conflicts=%s\n, conflicts);
 +
 +fprintf(f,
 +\n[Service]\n
 +Type=forking\n
 +Restart=no\n
 +TimeoutSec=5min\n
 +IgnoreSIGPIPE=no\n
 +KillMode=process\n
 +GuessMainPID=no\n
 +RemainAfterExit=%s\n,
 +s-pid_file ? no : yes);

 Maybe use yes_no(!s-pid_file) for this? it's a macro that does
 exactly this ternary operator check.

 +
 +if (s-sysv_start_priority  0)
 +fprintf(f, SysVStartPriority=%d\n, 
 s-sysv_start_priority);
 +
 + if (s-pid_file)
 + fprintf(f, PidFile=%s\n, s-pid_file);
 +
 + fprintf(f, ExecStart=%s start\n, s-path);
 + fprintf(f, ExecStop=%s stop\n, s-path);

 Please merge these two lines in one fprintf() invocation, like we do it
 for the other cases...

 + if (s-reload)
 + fprintf(f, ExecReload=%s reload\n, s-path);
 +
 +STRV_FOREACH(p, s-wants) {

 Looks like a whitespace issue... Only spaces please.

 +service = new0(SysvStub, 1);
 +if (!service)
 +return log_oom();
 +
 +service-sysv_start_priority = -1;
 +service-name = name;
 +service-path = fpath;
 +
 +hashmap_put(all_services, service-name,
 service);

 This can fail, due to OOM, we need to guard for this...


 +r = lookup_paths_init(
 +lp, SYSTEMD_SYSTEM, true,
 +NULL, NULL, NULL, NULL);

 Instead of making this a global variable, I'd prefer we could just pass
 that through as arguments to the functions that need this.

 Looks good otherwise!

Thanks. Pushed with fixes.

 Thanks!

 Lennart

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

[systemd-devel] [PATCH] udev: check the return value from udev_enumerate_scan_devices

[Thomas H.P. Andersen]

From: Thomas Hindoe Paaboel Andersen pho...@gmail.com

The return value from udev_enumerate_scan_devices was stored but
never used. I assume this was meant to be checked.
---
 src/udev/udevd.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/src/udev/udevd.c b/src/udev/udevd.c
index 0f3f3f0..f1daf49 100644
--- a/src/udev/udevd.c
+++ b/src/udev/udevd.c
@@ -780,6 +780,8 @@ static int synthesize_change(struct udev_device *dev) {
 return r;
 
 r = udev_enumerate_scan_devices(e);
+if (r  0)
+return r;
 
 udev_list_entry_foreach(item, 
udev_enumerate_get_list_entry(e)) {
 _cleanup_udev_device_unref_ struct udev_device *d = 
NULL;
-- 
1.9.3

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

Re: [systemd-devel] [PATCH v3 0/2] Move initscript parsing to a generator

[Thomas H.P. Andersen]

This needs some more work. Currently only initscripts linked from the rc?d
dirs are generated. We of course need to generate them all and have a way
to run the generator after a new initscript is installed.

Or do we want to run it just in time instead?

On May 30, 2014 3:28 PM, Thomas H.P. Andersen pho...@gmail.com wrote:

 From: Thomas Hindoe Paaboel Andersen pho...@gmail.com

 Move initscript parsing to a generator

 Compared to the previous version this one includes a fix for initscripts
that
 have no start priority. I have also updated the commit messages. The
patches
 have a few warts that I consider irrelevant but am willing to fix if
there is
 interest.

 The following now unused sysv-specific info are dropped from service dump:
  * SysV Init Script has LSB Header: (yes/no)
  * SysVEnabled: (yes/no)
  * SysVRunLevels: (levels)

 Note that this drops reading of chkconfig entirely. It also drops reading
 runlevels from the LSB headers. The runlevels were only used to check for
 runlevels outside of the normal 1-5 range and then add special
dependencies
 and settings. Special runlevels were dropped in the past so it seemed to
be
 unused code.

 The generator does not know about non-generated units with a value set
with
 SysVStartPriority=. These are therefor not taken into account when
converting
 start priority to before/after. After the special runlevels were dropped I
 don't see how this option adds any value.

 Thomas Hindoe Paaboel Andersen (2):
   Move handling of sysv initscripts to a generator
   Remove sysv parser from service.c

  .gitignore  |   1 +
  Makefile.am |  10 +
  src/core/service.c  | 984
+---
  src/core/service.h  |   5 -
  src/sysv-generator/Makefile |   1 +
  src/sysv-generator/sysv-generator.c | 899

  6 files changed, 921 insertions(+), 979 deletions(-)
  create mode 100644 src/sysv-generator/Makefile
  create mode 100644 src/sysv-generator/sysv-generator.c

 --
 1.9.3

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

Re: [systemd-devel] [PATCH v3 0/2] Move initscript parsing to a generator

[Thomas H.P. Andersen]

On Sat, May 31, 2014 at 11:34 AM, Tom Gundersen t...@jklm.no wrote:

 On 31 May 2014 08:39, Thomas H.P. Andersen pho...@gmail.com wrote:

 This needs some more work. Currently only initscripts linked from the rc?d
 dirs are generated. We of course need to generate them all and have a way to
 run the generator after a new initscript is installed.

 Or do we want to run it just in time instead?

 All generators are run on daemon-reload, which I think should be sufficient
 for this one too.

Thanks Tom.

I will rework the generator to create a .service for all scipts in
sysvinit path that have execute permission. After that it will adjust
the before/after/wants/conflicts based on the rc.d's.
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

[systemd-devel] [PATCH v4 0/2] Move initscript parsing to a generator

[Thomas H.P. Andersen]

From: Thomas Hindoe Paaboel Andersen pho...@gmail.com

Compared to the previous version this one generates a .service file for
all initscripts in the sysvinit path that have execute permission. After
that it will adjust the before/after/wants/conflicts based on the rc.d's.

The patches have a few warts that I consider irrelevant but am willing to
fix if there is interest:

The following now unused sysv-specific info are dropped from service dump:
 * SysV Init Script has LSB Header: (yes/no)
 * SysVEnabled: (yes/no)
 * SysVRunLevels: (levels)

Note that this drops reading of chkconfig entirely. It also drops reading
runlevels from the LSB headers. The runlevels were only used to check for
runlevels outside of the normal 1-5 range and then add special dependencies
and settings. Special runlevels were dropped in the past so it seemed to be
unused code.

The generator does not know about non-generated units with a value set with
SysVStartPriority=. These are therefor not taken into account when converting
start priority to before/after. After the special runlevels were dropped I
don't see how this option adds any value.

Thomas Hindoe Paaboel Andersen (2):
  Move handling of sysv initscripts to a generator
  Remove sysv parser from service.c

 .gitignore  |   1 +
 Makefile.am |  10 +
 src/core/service.c  | 984 +---
 src/core/service.h  |   5 -
 src/sysv-generator/Makefile |   1 +
 src/sysv-generator/sysv-generator.c | 909 +
 6 files changed, 931 insertions(+), 979 deletions(-)
 create mode 100644 src/sysv-generator/Makefile
 create mode 100644 src/sysv-generator/sysv-generator.c

-- 
1.9.3

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

[systemd-devel] [PATCH v4 1/2] Move handling of sysv initscripts to a generator

[Thomas H.P. Andersen]

From: Thomas Hindoe Paaboel Andersen pho...@gmail.com

Reuses logic from service.c and the rc-local generator.

Note that this drops reading of chkconfig entirely. It also drops reading
runlevels from the LSB headers. The runlevels were only used to check for
runlevels outside of the normal 1-5 range and then add special dependencies
and settings. Special runlevels were dropped in the past so it seemed to be
unused code.

The generator does not know about non-generated units with a value set with
SysVStartPriority=. These are therefor not taken into account when converting
start priority to before/after.
---
 .gitignore  |   1 +
 Makefile.am |  10 +
 src/sysv-generator/Makefile |   1 +
 src/sysv-generator/sysv-generator.c | 909 
 4 files changed, 921 insertions(+)
 create mode 100644 src/sysv-generator/Makefile
 create mode 100644 src/sysv-generator/sysv-generator.c

diff --git a/.gitignore b/.gitignore
index 908c563..061b4af 100644
--- a/.gitignore
+++ b/.gitignore
@@ -101,6 +101,7 @@
 /systemd-socket-proxyd
 /systemd-sysctl
 /systemd-system-update-generator
+/systemd-sysv-generator
 /systemd-timedated
 /systemd-timesyncd
 /systemd-tmpfiles
diff --git a/Makefile.am b/Makefile.am
index f66ef42..69d5a84 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -549,6 +549,7 @@ nodist_systemunit_DATA += \
units/halt-local.service
 
 systemgenerator_PROGRAMS += \
+   systemd-sysv-generator \
systemd-rc-local-generator
 endif
 
@@ -1915,6 +1916,15 @@ UNINSTALL_EXEC_HOOKS += dbus1-generator-uninstall-hook
 endif
 
 # 
--
+systemd_sysv_generator_SOURCES = \
+   src/sysv-generator/sysv-generator.c
+
+systemd_sysv_generator_LDADD = \
+   libsystemd-core.la \
+   libsystemd-label.la \
+   libsystemd-shared.la
+
+# 
--
 systemd_rc_local_generator_SOURCES = \
src/rc-local-generator/rc-local-generator.c
 
diff --git a/src/sysv-generator/Makefile b/src/sysv-generator/Makefile
new file mode 100644
index 000..530e5e9
--- /dev/null
+++ b/src/sysv-generator/Makefile
@@ -0,0 +1 @@
+../Makefile
diff --git a/src/sysv-generator/sysv-generator.c 
b/src/sysv-generator/sysv-generator.c
new file mode 100644
index 000..8351b4b
--- /dev/null
+++ b/src/sysv-generator/sysv-generator.c
@@ -0,0 +1,909 @@
+/*-*- Mode: C; c-basic-offset: 8; indent-tabs-mode: nil -*-*/
+
+/***
+  This file is part of systemd.
+
+  Copyright 2014 Thomas H.P. Andersen
+  Copyright 2010 Lennart Poettering
+  Copyright 2011 Michal Schmidt
+
+  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/.
+***/
+
+#include errno.h
+#include stdio.h
+#include unistd.h
+
+#include util.h
+#include mkdir.h
+#include strv.h
+#include path-util.h
+#include path-lookup.h
+#include log.h
+#include strv.h
+#include unit.h
+#include unit-name.h
+#include special.h
+#include exit-status.h
+#include def.h
+#include env-util.h
+#include fileio.h
+#include hashmap.h
+
+typedef enum RunlevelType {
+RUNLEVEL_UP,
+RUNLEVEL_DOWN
+} RunlevelType;
+
+static const struct {
+const char *path;
+const char *target;
+const RunlevelType type;
+} rcnd_table[] = {
+/* Standard SysV runlevels for start-up */
+{ rc1.d,  SPECIAL_RESCUE_TARGET,RUNLEVEL_UP },
+{ rc2.d,  SPECIAL_RUNLEVEL2_TARGET, RUNLEVEL_UP },
+{ rc3.d,  SPECIAL_RUNLEVEL3_TARGET, RUNLEVEL_UP },
+{ rc4.d,  SPECIAL_RUNLEVEL4_TARGET, RUNLEVEL_UP },
+{ rc5.d,  SPECIAL_RUNLEVEL5_TARGET, RUNLEVEL_UP },
+
+/* Standard SysV runlevels for shutdown */
+{ rc0.d,  SPECIAL_POWEROFF_TARGET,  RUNLEVEL_DOWN },
+{ rc6.d,  SPECIAL_REBOOT_TARGET,RUNLEVEL_DOWN }
+
+/* Note that the order here matters, as we read the
+   directories in this order, and we want to make sure that
+   sysv_start_priority is known when we first load the
+   unit. And that value we only know from S links. Hence
+   UP must be read before DOWN */
+};
+
+typedef struct SysvStub {
+char *name;
+char *path;
+char *description;
+int sysv_start_priority;
+char *pid_file;
+char **before;
+char **after

[systemd-devel] [PATCH v4 2/2] Remove sysv parser from service.c

[Thomas H.P. Andersen]

From: Thomas Hindoe Paaboel Andersen pho...@gmail.com

Parsing sysv files was moved to the sysv-generator in the previous commit.
This patch removes the sysv parsing from serivce.c.

To avoid introducing an extra compat option in .service fies for identifying
sysv services we instead add a check if the SourcePath is a path to sysvinit.

Note that this patch drops the following now unused sysv-specific info
from service dump:
SysV Init Script has LSB Header: (yes/no)
SysVEnabled: (yes/no)
SysVRunLevels: (levels)
---
 src/core/service.c | 984 +
 src/core/service.h |   5 -
 2 files changed, 10 insertions(+), 979 deletions(-)

diff --git a/src/core/service.c b/src/core/service.c
index 70e7635..836ca43 100644
--- a/src/core/service.c
+++ b/src/core/service.c
@@ -49,41 +49,6 @@
 #include bus-error.h
 #include bus-util.h
 
-#ifdef HAVE_SYSV_COMPAT
-
-#define DEFAULT_SYSV_TIMEOUT_USEC (5*USEC_PER_MINUTE)
-
-typedef enum RunlevelType {
-RUNLEVEL_UP,
-RUNLEVEL_DOWN
-} RunlevelType;
-
-static const struct {
-const char *path;
-const char *target;
-const RunlevelType type;
-} rcnd_table[] = {
-/* Standard SysV runlevels for start-up */
-{ rc1.d,  SPECIAL_RESCUE_TARGET,RUNLEVEL_UP },
-{ rc2.d,  SPECIAL_RUNLEVEL2_TARGET, RUNLEVEL_UP },
-{ rc3.d,  SPECIAL_RUNLEVEL3_TARGET, RUNLEVEL_UP },
-{ rc4.d,  SPECIAL_RUNLEVEL4_TARGET, RUNLEVEL_UP },
-{ rc5.d,  SPECIAL_RUNLEVEL5_TARGET, RUNLEVEL_UP },
-
-/* Standard SysV runlevels for shutdown */
-{ rc0.d,  SPECIAL_POWEROFF_TARGET,  RUNLEVEL_DOWN },
-{ rc6.d,  SPECIAL_REBOOT_TARGET,RUNLEVEL_DOWN }
-
-/* Note that the order here matters, as we read the
-   directories in this order, and we want to make sure that
-   sysv_start_priority is known when we first load the
-   unit. And that value we only know from S links. Hence
-   UP must be read before DOWN */
-};
-
-#define RUNLEVELS_UP 12345
-#endif
-
 static const UnitActiveState state_translation_table[_SERVICE_STATE_MAX] = {
 [SERVICE_DEAD] = UNIT_INACTIVE,
 [SERVICE_START_PRE] = UNIT_ACTIVATING,
@@ -138,11 +103,6 @@ static void service_init(Unit *u) {
 s-timeout_stop_usec = u-manager-default_timeout_stop_usec;
 s-restart_usec = u-manager-default_restart_usec;
 s-type = _SERVICE_TYPE_INVALID;
-
-#ifdef HAVE_SYSV_COMPAT
-s-sysv_start_priority = -1;
-s-sysv_start_priority_from_rcnd = -1;
-#endif
 s-socket_fd = -1;
 s-guess_main_pid = true;
 
@@ -295,11 +255,6 @@ static void service_done(Unit *u) {
 free(s-pid_file);
 s-pid_file = NULL;
 
-#ifdef HAVE_SYSV_COMPAT
-free(s-sysv_runlevels);
-s-sysv_runlevels = NULL;
-#endif
-
 free(s-status_text);
 s-status_text = NULL;
 
@@ -364,734 +319,6 @@ static int service_arm_timer(Service *s, usec_t usec) {
 service_dispatch_timer, s);
 }
 
-#ifdef HAVE_SYSV_COMPAT
-static char *sysv_translate_name(const char *name) {
-char *r;
-
-r = new(char, strlen(name) + strlen(.service) + 1);
-if (!r)
-return NULL;
-
-if (endswith(name, .sh))
-/* Drop .sh suffix */
-strcpy(stpcpy(r, name) - 3, .service);
-else
-/* Normal init script name */
-strcpy(stpcpy(r, name), .service);
-
-return r;
-}
-
-static int sysv_translate_facility(const char *name, const char *filename, 
char **_r) {
-
-/* We silently ignore the $ prefix here. According to the LSB
- * spec it simply indicates whether something is a
- * standardized name or a distribution-specific one. Since we
- * just follow what already exists and do not introduce new
- * uses or names we don't care who introduced a new name. */
-
-static const char * const table[] = {
-/* LSB defined facilities */
-local_fs, NULL,
-network,  SPECIAL_NETWORK_ONLINE_TARGET,
-named,SPECIAL_NSS_LOOKUP_TARGET,
-portmap,  SPECIAL_RPCBIND_TARGET,
-remote_fs,SPECIAL_REMOTE_FS_TARGET,
-syslog,   NULL,
-time, SPECIAL_TIME_SYNC_TARGET,
-};
-
-unsigned i;
-char *r;
-const char *n;
-
-assert(name);
-assert(_r);
-
-n = *name == '$' ? name + 1 : name;
-
-for (i = 0; i  ELEMENTSOF(table); i += 2) {
-
-if (!streq(table[i], n))
-continue;
-
-if (!table[i+1])
-return 0;
-
-r = strdup(table[i+1]);
-if (!r)
-return log_oom();
-
-goto finish;

[systemd-devel] [PATCH v3 0/2] Move initscript parsing to a generator

[Thomas H.P. Andersen]

From: Thomas Hindoe Paaboel Andersen pho...@gmail.com

Move initscript parsing to a generator

Compared to the previous version this one includes a fix for initscripts that
have no start priority. I have also updated the commit messages. The patches
have a few warts that I consider irrelevant but am willing to fix if there is
interest.

The following now unused sysv-specific info are dropped from service dump:
 * SysV Init Script has LSB Header: (yes/no)
 * SysVEnabled: (yes/no)
 * SysVRunLevels: (levels)

Note that this drops reading of chkconfig entirely. It also drops reading
runlevels from the LSB headers. The runlevels were only used to check for
runlevels outside of the normal 1-5 range and then add special dependencies
and settings. Special runlevels were dropped in the past so it seemed to be
unused code.

The generator does not know about non-generated units with a value set with
SysVStartPriority=. These are therefor not taken into account when converting
start priority to before/after. After the special runlevels were dropped I 
don't see how this option adds any value.

Thomas Hindoe Paaboel Andersen (2):
  Move handling of sysv initscripts to a generator
  Remove sysv parser from service.c

 .gitignore  |   1 +
 Makefile.am |  10 +
 src/core/service.c  | 984 +---
 src/core/service.h  |   5 -
 src/sysv-generator/Makefile |   1 +
 src/sysv-generator/sysv-generator.c | 899 
 6 files changed, 921 insertions(+), 979 deletions(-)
 create mode 100644 src/sysv-generator/Makefile
 create mode 100644 src/sysv-generator/sysv-generator.c

-- 
1.9.3

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

[systemd-devel] [PATCH v3 2/2] Remove sysv parser from service.c

[Thomas H.P. Andersen]

From: Thomas Hindoe Paaboel Andersen pho...@gmail.com

Parsing sysv files was moved to the sysv-generator in the previous commit.
This patch removes the sysv parsing from serivce.c.

To avoid introducing an extra compat option in .service fies for identifying
sysv services we instead add a check if the SourcePath is a path to sysvinit.

Note that this patch drops the following now unused sysv-specific info
from service dump:
SysV Init Script has LSB Header: (yes/no)
SysVEnabled: (yes/no)
SysVRunLevels: (levels)
---
 src/core/service.c | 984 +
 src/core/service.h |   5 -
 2 files changed, 10 insertions(+), 979 deletions(-)

diff --git a/src/core/service.c b/src/core/service.c
index 70e7635..836ca43 100644
--- a/src/core/service.c
+++ b/src/core/service.c
@@ -49,41 +49,6 @@
 #include bus-error.h
 #include bus-util.h
 
-#ifdef HAVE_SYSV_COMPAT
-
-#define DEFAULT_SYSV_TIMEOUT_USEC (5*USEC_PER_MINUTE)
-
-typedef enum RunlevelType {
-RUNLEVEL_UP,
-RUNLEVEL_DOWN
-} RunlevelType;
-
-static const struct {
-const char *path;
-const char *target;
-const RunlevelType type;
-} rcnd_table[] = {
-/* Standard SysV runlevels for start-up */
-{ rc1.d,  SPECIAL_RESCUE_TARGET,RUNLEVEL_UP },
-{ rc2.d,  SPECIAL_RUNLEVEL2_TARGET, RUNLEVEL_UP },
-{ rc3.d,  SPECIAL_RUNLEVEL3_TARGET, RUNLEVEL_UP },
-{ rc4.d,  SPECIAL_RUNLEVEL4_TARGET, RUNLEVEL_UP },
-{ rc5.d,  SPECIAL_RUNLEVEL5_TARGET, RUNLEVEL_UP },
-
-/* Standard SysV runlevels for shutdown */
-{ rc0.d,  SPECIAL_POWEROFF_TARGET,  RUNLEVEL_DOWN },
-{ rc6.d,  SPECIAL_REBOOT_TARGET,RUNLEVEL_DOWN }
-
-/* Note that the order here matters, as we read the
-   directories in this order, and we want to make sure that
-   sysv_start_priority is known when we first load the
-   unit. And that value we only know from S links. Hence
-   UP must be read before DOWN */
-};
-
-#define RUNLEVELS_UP 12345
-#endif
-
 static const UnitActiveState state_translation_table[_SERVICE_STATE_MAX] = {
 [SERVICE_DEAD] = UNIT_INACTIVE,
 [SERVICE_START_PRE] = UNIT_ACTIVATING,
@@ -138,11 +103,6 @@ static void service_init(Unit *u) {
 s-timeout_stop_usec = u-manager-default_timeout_stop_usec;
 s-restart_usec = u-manager-default_restart_usec;
 s-type = _SERVICE_TYPE_INVALID;
-
-#ifdef HAVE_SYSV_COMPAT
-s-sysv_start_priority = -1;
-s-sysv_start_priority_from_rcnd = -1;
-#endif
 s-socket_fd = -1;
 s-guess_main_pid = true;
 
@@ -295,11 +255,6 @@ static void service_done(Unit *u) {
 free(s-pid_file);
 s-pid_file = NULL;
 
-#ifdef HAVE_SYSV_COMPAT
-free(s-sysv_runlevels);
-s-sysv_runlevels = NULL;
-#endif
-
 free(s-status_text);
 s-status_text = NULL;
 
@@ -364,734 +319,6 @@ static int service_arm_timer(Service *s, usec_t usec) {
 service_dispatch_timer, s);
 }
 
-#ifdef HAVE_SYSV_COMPAT
-static char *sysv_translate_name(const char *name) {
-char *r;
-
-r = new(char, strlen(name) + strlen(.service) + 1);
-if (!r)
-return NULL;
-
-if (endswith(name, .sh))
-/* Drop .sh suffix */
-strcpy(stpcpy(r, name) - 3, .service);
-else
-/* Normal init script name */
-strcpy(stpcpy(r, name), .service);
-
-return r;
-}
-
-static int sysv_translate_facility(const char *name, const char *filename, 
char **_r) {
-
-/* We silently ignore the $ prefix here. According to the LSB
- * spec it simply indicates whether something is a
- * standardized name or a distribution-specific one. Since we
- * just follow what already exists and do not introduce new
- * uses or names we don't care who introduced a new name. */
-
-static const char * const table[] = {
-/* LSB defined facilities */
-local_fs, NULL,
-network,  SPECIAL_NETWORK_ONLINE_TARGET,
-named,SPECIAL_NSS_LOOKUP_TARGET,
-portmap,  SPECIAL_RPCBIND_TARGET,
-remote_fs,SPECIAL_REMOTE_FS_TARGET,
-syslog,   NULL,
-time, SPECIAL_TIME_SYNC_TARGET,
-};
-
-unsigned i;
-char *r;
-const char *n;
-
-assert(name);
-assert(_r);
-
-n = *name == '$' ? name + 1 : name;
-
-for (i = 0; i  ELEMENTSOF(table); i += 2) {
-
-if (!streq(table[i], n))
-continue;
-
-if (!table[i+1])
-return 0;
-
-r = strdup(table[i+1]);
-if (!r)
-return log_oom();
-
-goto finish;

[systemd-devel] [PATCH v3 1/2] Move handling of sysv initscripts to a generator

[Thomas H.P. Andersen]

From: Thomas Hindoe Paaboel Andersen pho...@gmail.com

Reuses logic from service.c and the rc-local generator.

Note that this drops reading of chkconfig entirely. It also drops reading
runlevels from the LSB headers. The runlevels were only used to check for
runlevels outside of the normal 1-5 range and then add special dependencies
and settings. Special runlevels were dropped in the past so it seemed to be
unused code.

The generator does not know about non-generated units with a value set with
SysVStartPriority=. These are therefor not taken into account when converting
start priority to before/after.
---
 .gitignore  |   1 +
 Makefile.am |  10 +
 src/sysv-generator/Makefile |   1 +
 src/sysv-generator/sysv-generator.c | 899 
 4 files changed, 911 insertions(+)
 create mode 100644 src/sysv-generator/Makefile
 create mode 100644 src/sysv-generator/sysv-generator.c

diff --git a/.gitignore b/.gitignore
index 908c563..061b4af 100644
--- a/.gitignore
+++ b/.gitignore
@@ -101,6 +101,7 @@
 /systemd-socket-proxyd
 /systemd-sysctl
 /systemd-system-update-generator
+/systemd-sysv-generator
 /systemd-timedated
 /systemd-timesyncd
 /systemd-tmpfiles
diff --git a/Makefile.am b/Makefile.am
index f66ef42..69d5a84 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -549,6 +549,7 @@ nodist_systemunit_DATA += \
units/halt-local.service
 
 systemgenerator_PROGRAMS += \
+   systemd-sysv-generator \
systemd-rc-local-generator
 endif
 
@@ -1915,6 +1916,15 @@ UNINSTALL_EXEC_HOOKS += dbus1-generator-uninstall-hook
 endif
 
 # 
--
+systemd_sysv_generator_SOURCES = \
+   src/sysv-generator/sysv-generator.c
+
+systemd_sysv_generator_LDADD = \
+   libsystemd-core.la \
+   libsystemd-label.la \
+   libsystemd-shared.la
+
+# 
--
 systemd_rc_local_generator_SOURCES = \
src/rc-local-generator/rc-local-generator.c
 
diff --git a/src/sysv-generator/Makefile b/src/sysv-generator/Makefile
new file mode 100644
index 000..530e5e9
--- /dev/null
+++ b/src/sysv-generator/Makefile
@@ -0,0 +1 @@
+../Makefile
diff --git a/src/sysv-generator/sysv-generator.c 
b/src/sysv-generator/sysv-generator.c
new file mode 100644
index 000..565fcaa
--- /dev/null
+++ b/src/sysv-generator/sysv-generator.c
@@ -0,0 +1,899 @@
+/*-*- Mode: C; c-basic-offset: 8; indent-tabs-mode: nil -*-*/
+
+/***
+  This file is part of systemd.
+
+  Copyright 2014 Thomas H.P. Andersen
+  Copyright 2010 Lennart Poettering
+  Copyright 2011 Michal Schmidt
+
+  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/.
+***/
+
+#include errno.h
+#include stdio.h
+#include unistd.h
+
+#include util.h
+#include mkdir.h
+#include strv.h
+#include path-util.h
+#include path-lookup.h
+#include log.h
+#include strv.h
+#include unit.h
+#include unit-name.h
+#include special.h
+#include exit-status.h
+#include def.h
+#include env-util.h
+#include fileio.h
+#include hashmap.h
+
+typedef enum RunlevelType {
+RUNLEVEL_UP,
+RUNLEVEL_DOWN
+} RunlevelType;
+
+static const struct {
+const char *path;
+const char *target;
+const RunlevelType type;
+} rcnd_table[] = {
+/* Standard SysV runlevels for start-up */
+{ rc1.d,  SPECIAL_RESCUE_TARGET,RUNLEVEL_UP },
+{ rc2.d,  SPECIAL_RUNLEVEL2_TARGET, RUNLEVEL_UP },
+{ rc3.d,  SPECIAL_RUNLEVEL3_TARGET, RUNLEVEL_UP },
+{ rc4.d,  SPECIAL_RUNLEVEL4_TARGET, RUNLEVEL_UP },
+{ rc5.d,  SPECIAL_RUNLEVEL5_TARGET, RUNLEVEL_UP },
+
+/* Standard SysV runlevels for shutdown */
+{ rc0.d,  SPECIAL_POWEROFF_TARGET,  RUNLEVEL_DOWN },
+{ rc6.d,  SPECIAL_REBOOT_TARGET,RUNLEVEL_DOWN }
+
+/* Note that the order here matters, as we read the
+   directories in this order, and we want to make sure that
+   sysv_start_priority is known when we first load the
+   unit. And that value we only know from S links. Hence
+   UP must be read before DOWN */
+};
+
+typedef struct SysvStub {
+char *name;
+char *path;
+char *description;
+int sysv_start_priority;
+char *pid_file;
+char **before;
+char **after

Re: [systemd-devel] [PATCH] [RFC] Move handling of sysv initscripts to a generator

[Thomas H.P. Andersen]

On Wed, May 28, 2014 at 3:38 AM, Zbigniew Jędrzejewski-Szmek
zbys...@in.waw.pl wrote:
 On Wed, May 28, 2014 at 01:12:23AM +0200, Thomas H.P. Andersen wrote:
 From: Thomas Hindoe Paaboel Andersen pho...@gmail.com

 Reuses logic from service.c and the rc-local generator.

 Note that this drops reading of chkconfig entirely.
 How likely is this to cause regressions in existing distributions?

 It also drops reading
 runlevels from the LSB headers. The runlevels were only used to check for
 runlevels outside of the normal 1-5 range and then add special dependencies
 and settings. Special runlevels were dropped in the past so it seemed to be
 unused code.

 Also note that this code behaves differently to the old if an initcsript
 and native unit exist with the same name. Before the initscript would be
 ignored but now a .service file is created in /run which will override
 the native unit.
 This is a total no-no. This would immediately break existing setups,
 becuase since forever this has been documented and advertised as a
 compatibility mechanism.

 The old code dealing with initscripts are left in for now as the generator
 will run first and the old code will then ignore the initscripts since
 unit files exist for them. I will add another patch to rip the old code out
 later.
 It would be nice to have this counterpart too, since then it would be easier
 to tell how much complexity and existing code we are removing. I think that
 there's general agreement to the idea of moving sysv support to a generator,
 the question is only if we can do it without significant breakage.

I cleaned up the code for yours and Peeters comments. Units generated
from initscripts no longer take precedence over native units.

Ripping out the old code was harder than I thought though. The Service
struct contains a few sysv-specific fields that were filled directly
when the initscripts were parsed. Only one of them can be set from a
.service file. If we want to keep them all then we need to add options
for them in the .service file. IMO all but one are irrelevant with the
generator. The relevant one left is is_sysv. It is used to prevent the
sysv unit from garbage collection and to refuse socket activation for
sysv services. The remaining fields (sysv_has_lsb, sysv_enabled,
sysv_start_priority_from_rcnd, sysv_start_priority, sysv_runlevels)
are now only used in service_dump and I cannot see any code in systemd
making use of the dumped values.

Would it be acceptable to just drop the fields only used in service_dump?
Is it okay to add a SysV=true/false option to .service?
What about the SysVStartPriority= option set in native .service's?
The generator does not know about non-generated units with such a
value set so it cannot take them into account when converting start
priority to before/after. Should the manager itself try to reorder all
units with a sysv priority later?

 The patch is WIP and only tested lightly. I am mostly looking for comments
 if this is a good idea at all.
 The code look OK.

 ---
  .gitignore  |   1 +
  Makefile.am |  10 +
  src/sysv-generator/Makefile |   1 +
  src/sysv-generator/sysv-generator.c | 896 
 
  4 files changed, 908 insertions(+)
  create mode 100644 src/sysv-generator/Makefile
  create mode 100644 src/sysv-generator/sysv-generator.c

 diff --git a/.gitignore b/.gitignore
 index 908c563..061b4af 100644
 --- a/.gitignore
 +++ b/.gitignore
 @@ -101,6 +101,7 @@
  /systemd-socket-proxyd
  /systemd-sysctl
  /systemd-system-update-generator
 +/systemd-sysv-generator
  /systemd-timedated
  /systemd-timesyncd
  /systemd-tmpfiles
 diff --git a/Makefile.am b/Makefile.am
 index 5b26bc3..a395969 100644
 --- a/Makefile.am
 +++ b/Makefile.am
 @@ -549,6 +549,7 @@ nodist_systemunit_DATA += \
   units/halt-local.service

  systemgenerator_PROGRAMS += \
 + systemd-sysv-generator \
   systemd-rc-local-generator
  endif

 @@ -1918,6 +1919,15 @@ UNINSTALL_EXEC_HOOKS += dbus1-generator-uninstall-hook
  endif

  # 
 --
 +systemd_sysv_generator_SOURCES = \
 + src/sysv-generator/sysv-generator.c
 +
 +systemd_sysv_generator_LDADD = \
 + libsystemd-core.la \
 + libsystemd-label.la \
 + libsystemd-shared.la
 +
 +# 
 --
  systemd_rc_local_generator_SOURCES = \
   src/rc-local-generator/rc-local-generator.c

 diff --git a/src/sysv-generator/Makefile b/src/sysv-generator/Makefile
 new file mode 100644
 index 000..530e5e9
 --- /dev/null
 +++ b/src/sysv-generator/Makefile
 @@ -0,0 +1 @@
 +../Makefile
 diff --git a/src/sysv-generator/sysv-generator.c 
 b/src/sysv-generator/sysv-generator.c
 new file mode 100644
 index 000..7d153a6
 --- /dev/null
 +++ b/src/sysv-generator/sysv-generator.c
 @@ -0,0 +1,896 @@
 +/*-*- Mode: C; c-basic-offset: 8; indent

Re: [systemd-devel] [PATCH] [RFC] Move handling of sysv initscripts to a generator

[Thomas H.P. Andersen]

On Thu, May 29, 2014 at 4:30 PM, Andrey Borzenkov arvidj...@gmail.com wrote:
 В Thu, 29 May 2014 16:11:25 +0200
 Thomas H.P. Andersen pho...@gmail.com пишет:

 On Wed, May 28, 2014 at 3:38 AM, Zbigniew Jędrzejewski-Szmek
 zbys...@in.waw.pl wrote:
  On Wed, May 28, 2014 at 01:12:23AM +0200, Thomas H.P. Andersen wrote:
  From: Thomas Hindoe Paaboel Andersen pho...@gmail.com
 
  Reuses logic from service.c and the rc-local generator.
 
  Note that this drops reading of chkconfig entirely.
  How likely is this to cause regressions in existing distributions?
 
  It also drops reading
  runlevels from the LSB headers. The runlevels were only used to check for
  runlevels outside of the normal 1-5 range and then add special 
  dependencies
  and settings. Special runlevels were dropped in the past so it seemed to 
  be
  unused code.
 
  Also note that this code behaves differently to the old if an initcsript
  and native unit exist with the same name. Before the initscript would be
  ignored but now a .service file is created in /run which will override
  the native unit.
  This is a total no-no. This would immediately break existing setups,
  becuase since forever this has been documented and advertised as a
  compatibility mechanism.
 
  The old code dealing with initscripts are left in for now as the generator
  will run first and the old code will then ignore the initscripts since
  unit files exist for them. I will add another patch to rip the old code 
  out
  later.
  It would be nice to have this counterpart too, since then it would be 
  easier
  to tell how much complexity and existing code we are removing. I think that
  there's general agreement to the idea of moving sysv support to a 
  generator,
  the question is only if we can do it without significant breakage.

 I cleaned up the code for yours and Peeters comments. Units generated
 from initscripts no longer take precedence over native units.

 Ripping out the old code was harder than I thought though. The Service
 struct contains a few sysv-specific fields that were filled directly
 when the initscripts were parsed. Only one of them can be set from a
 .service file. If we want to keep them all then we need to add options
 for them in the .service file. IMO all but one are irrelevant with the
 generator. The relevant one left is is_sysv. It is used to prevent the
 sysv unit from garbage collection and to refuse socket activation for
 sysv services. The remaining fields (sysv_has_lsb, sysv_enabled,
 sysv_start_priority_from_rcnd, sysv_start_priority, sysv_runlevels)
 are now only used in service_dump and I cannot see any code in systemd
 making use of the dumped values.

 Would it be acceptable to just drop the fields only used in service_dump?
 Is it okay to add a SysV=true/false option to .service?

 Could it make use of SourcePath to avoid adding special case variable?

Not a bad idea. If SourcePath is within the sysvinit path then assume
a sysv service.

Here is the generated unit from livesys on f20 btw. We could go for
extra checks like if a is set Description it must start with eihter
LSB: or SYSV: . But SourcePath seems like a reliable way to me.

# Automatically generated by systemd-sysv-generator

[Unit]
SourcePath=/etc/rc.d/init.d/livesys
Description=LSB: Init script for live image.
Before=runlevel3.target runlevel4.target runlevel5.target
shutdown.target display-manager.service livesys-late.service
Conflicts=shutdown.target

[Service]
Type=forking
Restart=no
TimeoutSec=5min
IgnoreSIGPIPE=no
KillMode=process
GuessMainPID=no
SysVStartPriority=0
RemainAfterExit=yes
ExecStart=/etc/rc.d/init.d/livesys start
ExecStop=/etc/rc.d/init.d/livesys stop

 What about the SysVStartPriority= option set in native .service's?
 The generator does not know about non-generated units with such a
 value set so it cannot take them into account when converting start
 priority to before/after. Should the manager itself try to reorder all
 units with a sysv priority later?

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

[systemd-devel] [PATCH 2/2] Remove sysv parser from pid1

[Thomas H.P. Andersen]

From: Thomas Hindoe Paaboel Andersen pho...@gmail.com

Parsing sysv files was moved to the sysv-generator.

To avoid introducing an extra compat option for identifying sysv services
we instead add a check if the SourcePath is a path to sysvinit.

Note that this patch drops the following now unused sysv-specific info
from service dump:
SysV Init Script has LSB Header: (yes/no)
SysVEnabled: (yes/no)
SysVRunLevels: (levels)
---
 src/core/service.c | 984 +
 src/core/service.h |   5 -
 2 files changed, 10 insertions(+), 979 deletions(-)

diff --git a/src/core/service.c b/src/core/service.c
index 70e7635..836ca43 100644
--- a/src/core/service.c
+++ b/src/core/service.c
@@ -49,41 +49,6 @@
 #include bus-error.h
 #include bus-util.h
 
-#ifdef HAVE_SYSV_COMPAT
-
-#define DEFAULT_SYSV_TIMEOUT_USEC (5*USEC_PER_MINUTE)
-
-typedef enum RunlevelType {
-RUNLEVEL_UP,
-RUNLEVEL_DOWN
-} RunlevelType;
-
-static const struct {
-const char *path;
-const char *target;
-const RunlevelType type;
-} rcnd_table[] = {
-/* Standard SysV runlevels for start-up */
-{ rc1.d,  SPECIAL_RESCUE_TARGET,RUNLEVEL_UP },
-{ rc2.d,  SPECIAL_RUNLEVEL2_TARGET, RUNLEVEL_UP },
-{ rc3.d,  SPECIAL_RUNLEVEL3_TARGET, RUNLEVEL_UP },
-{ rc4.d,  SPECIAL_RUNLEVEL4_TARGET, RUNLEVEL_UP },
-{ rc5.d,  SPECIAL_RUNLEVEL5_TARGET, RUNLEVEL_UP },
-
-/* Standard SysV runlevels for shutdown */
-{ rc0.d,  SPECIAL_POWEROFF_TARGET,  RUNLEVEL_DOWN },
-{ rc6.d,  SPECIAL_REBOOT_TARGET,RUNLEVEL_DOWN }
-
-/* Note that the order here matters, as we read the
-   directories in this order, and we want to make sure that
-   sysv_start_priority is known when we first load the
-   unit. And that value we only know from S links. Hence
-   UP must be read before DOWN */
-};
-
-#define RUNLEVELS_UP 12345
-#endif
-
 static const UnitActiveState state_translation_table[_SERVICE_STATE_MAX] = {
 [SERVICE_DEAD] = UNIT_INACTIVE,
 [SERVICE_START_PRE] = UNIT_ACTIVATING,
@@ -138,11 +103,6 @@ static void service_init(Unit *u) {
 s-timeout_stop_usec = u-manager-default_timeout_stop_usec;
 s-restart_usec = u-manager-default_restart_usec;
 s-type = _SERVICE_TYPE_INVALID;
-
-#ifdef HAVE_SYSV_COMPAT
-s-sysv_start_priority = -1;
-s-sysv_start_priority_from_rcnd = -1;
-#endif
 s-socket_fd = -1;
 s-guess_main_pid = true;
 
@@ -295,11 +255,6 @@ static void service_done(Unit *u) {
 free(s-pid_file);
 s-pid_file = NULL;
 
-#ifdef HAVE_SYSV_COMPAT
-free(s-sysv_runlevels);
-s-sysv_runlevels = NULL;
-#endif
-
 free(s-status_text);
 s-status_text = NULL;
 
@@ -364,734 +319,6 @@ static int service_arm_timer(Service *s, usec_t usec) {
 service_dispatch_timer, s);
 }
 
-#ifdef HAVE_SYSV_COMPAT
-static char *sysv_translate_name(const char *name) {
-char *r;
-
-r = new(char, strlen(name) + strlen(.service) + 1);
-if (!r)
-return NULL;
-
-if (endswith(name, .sh))
-/* Drop .sh suffix */
-strcpy(stpcpy(r, name) - 3, .service);
-else
-/* Normal init script name */
-strcpy(stpcpy(r, name), .service);
-
-return r;
-}
-
-static int sysv_translate_facility(const char *name, const char *filename, 
char **_r) {
-
-/* We silently ignore the $ prefix here. According to the LSB
- * spec it simply indicates whether something is a
- * standardized name or a distribution-specific one. Since we
- * just follow what already exists and do not introduce new
- * uses or names we don't care who introduced a new name. */
-
-static const char * const table[] = {
-/* LSB defined facilities */
-local_fs, NULL,
-network,  SPECIAL_NETWORK_ONLINE_TARGET,
-named,SPECIAL_NSS_LOOKUP_TARGET,
-portmap,  SPECIAL_RPCBIND_TARGET,
-remote_fs,SPECIAL_REMOTE_FS_TARGET,
-syslog,   NULL,
-time, SPECIAL_TIME_SYNC_TARGET,
-};
-
-unsigned i;
-char *r;
-const char *n;
-
-assert(name);
-assert(_r);
-
-n = *name == '$' ? name + 1 : name;
-
-for (i = 0; i  ELEMENTSOF(table); i += 2) {
-
-if (!streq(table[i], n))
-continue;
-
-if (!table[i+1])
-return 0;
-
-r = strdup(table[i+1]);
-if (!r)
-return log_oom();
-
-goto finish;
-}
-
-/* If we don't know this name, fallback heuristics to figure
- 

[systemd-devel] [PATCH 1/2] [RFC] Move handling of sysv initscripts to a generator

[Thomas H.P. Andersen]

From: Thomas Hindoe Paaboel Andersen pho...@gmail.com

Reuses logic from service.c and the rc-local generator.

Note that this drops reading of chkconfig entirely. It also drops reading
runlevels from the LSB headers. The runlevels were only used to check for
runlevels outside of the normal 1-5 range and then add special dependencies
and settings. Special runlevels were dropped in the past so it seemed to be
unused code.

The patch is WIP and only tested lightly. I am mostly looking for comments
if this is a good idea at all.
---
 .gitignore  |   1 +
 Makefile.am |  10 +
 src/sysv-generator/Makefile |   1 +
 src/sysv-generator/sysv-generator.c | 898 
 4 files changed, 910 insertions(+)
 create mode 100644 src/sysv-generator/Makefile
 create mode 100644 src/sysv-generator/sysv-generator.c

diff --git a/.gitignore b/.gitignore
index 908c563..061b4af 100644
--- a/.gitignore
+++ b/.gitignore
@@ -101,6 +101,7 @@
 /systemd-socket-proxyd
 /systemd-sysctl
 /systemd-system-update-generator
+/systemd-sysv-generator
 /systemd-timedated
 /systemd-timesyncd
 /systemd-tmpfiles
diff --git a/Makefile.am b/Makefile.am
index f66ef42..69d5a84 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -549,6 +549,7 @@ nodist_systemunit_DATA += \
units/halt-local.service
 
 systemgenerator_PROGRAMS += \
+   systemd-sysv-generator \
systemd-rc-local-generator
 endif
 
@@ -1915,6 +1916,15 @@ UNINSTALL_EXEC_HOOKS += dbus1-generator-uninstall-hook
 endif
 
 # 
--
+systemd_sysv_generator_SOURCES = \
+   src/sysv-generator/sysv-generator.c
+
+systemd_sysv_generator_LDADD = \
+   libsystemd-core.la \
+   libsystemd-label.la \
+   libsystemd-shared.la
+
+# 
--
 systemd_rc_local_generator_SOURCES = \
src/rc-local-generator/rc-local-generator.c
 
diff --git a/src/sysv-generator/Makefile b/src/sysv-generator/Makefile
new file mode 100644
index 000..530e5e9
--- /dev/null
+++ b/src/sysv-generator/Makefile
@@ -0,0 +1 @@
+../Makefile
diff --git a/src/sysv-generator/sysv-generator.c 
b/src/sysv-generator/sysv-generator.c
new file mode 100644
index 000..9bdd8af
--- /dev/null
+++ b/src/sysv-generator/sysv-generator.c
@@ -0,0 +1,898 @@
+/*-*- Mode: C; c-basic-offset: 8; indent-tabs-mode: nil -*-*/
+
+/***
+  This file is part of systemd.
+
+  Copyright 2014 Thomas H.P. Andersen
+  Copyright 2010 Lennart Poettering
+  Copyright 2011 Michal Schmidt
+
+  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/.
+***/
+
+#include errno.h
+#include stdio.h
+#include unistd.h
+
+#include util.h
+#include mkdir.h
+#include strv.h
+#include path-util.h
+#include path-lookup.h
+#include log.h
+#include strv.h
+#include unit.h
+#include unit-name.h
+#include special.h
+#include exit-status.h
+#include def.h
+#include env-util.h
+#include fileio.h
+#include hashmap.h
+
+typedef enum RunlevelType {
+RUNLEVEL_UP,
+RUNLEVEL_DOWN
+} RunlevelType;
+
+static const struct {
+const char *path;
+const char *target;
+const RunlevelType type;
+} rcnd_table[] = {
+/* Standard SysV runlevels for start-up */
+{ rc1.d,  SPECIAL_RESCUE_TARGET,RUNLEVEL_UP },
+{ rc2.d,  SPECIAL_RUNLEVEL2_TARGET, RUNLEVEL_UP },
+{ rc3.d,  SPECIAL_RUNLEVEL3_TARGET, RUNLEVEL_UP },
+{ rc4.d,  SPECIAL_RUNLEVEL4_TARGET, RUNLEVEL_UP },
+{ rc5.d,  SPECIAL_RUNLEVEL5_TARGET, RUNLEVEL_UP },
+
+/* Standard SysV runlevels for shutdown */
+{ rc0.d,  SPECIAL_POWEROFF_TARGET,  RUNLEVEL_DOWN },
+{ rc6.d,  SPECIAL_REBOOT_TARGET,RUNLEVEL_DOWN }
+
+/* Note that the order here matters, as we read the
+   directories in this order, and we want to make sure that
+   sysv_start_priority is known when we first load the
+   unit. And that value we only know from S links. Hence
+   UP must be read before DOWN */
+};
+
+typedef struct SysvStub {
+char *name;
+char *path;
+char *description;
+int sysv_start_priority;
+char *pid_file;
+char **before;
+char **after;
+char **wants;
+char **conflicts;
+bool has_lsb;
+bool

Re: [systemd-devel] [PATCH] [RFC] Move handling of sysv initscripts to a generator

[Thomas H.P. Andersen]

On Wed, May 28, 2014 at 3:38 AM, Zbigniew Jędrzejewski-Szmek
zbys...@in.waw.pl wrote:
 On Wed, May 28, 2014 at 01:12:23AM +0200, Thomas H.P. Andersen wrote:
 From: Thomas Hindoe Paaboel Andersen pho...@gmail.com

 Reuses logic from service.c and the rc-local generator.

 Note that this drops reading of chkconfig entirely.
 How likely is this to cause regressions in existing distributions?
The start priority in chkconfig was already removed in 213 so the only
thing was the runlevels. The runlevels were only used to determine if
the runlevel was special (not 1-5). Since special runlevels was
dropped way back I don't see any reason to read the chkconfig at all.

 It also drops reading
 runlevels from the LSB headers. The runlevels were only used to check for
 runlevels outside of the normal 1-5 range and then add special dependencies
 and settings. Special runlevels were dropped in the past so it seemed to be
 unused code.

 Also note that this code behaves differently to the old if an initcsript
 and native unit exist with the same name. Before the initscript would be
 ignored but now a .service file is created in /run which will override
 the native unit.
 This is a total no-no. This would immediately break existing setups,
 becuase since forever this has been documented and advertised as a
 compatibility mechanism.
Definitely something to address before the patch is ready. I only
meant to highlight the issue in case anybody wanted to test it.

 The old code dealing with initscripts are left in for now as the generator
 will run first and the old code will then ignore the initscripts since
 unit files exist for them. I will add another patch to rip the old code out
 later.
 It would be nice to have this counterpart too, since then it would be easier
 to tell how much complexity and existing code we are removing. I think that
 there's general agreement to the idea of moving sysv support to a generator,
 the question is only if we can do it without significant breakage.

 The patch is WIP and only tested lightly. I am mostly looking for comments
 if this is a good idea at all.
 The code look OK.

Thanks Zbyszek. I will resend the patch tonight with fixes for your comments.

 ---
  .gitignore  |   1 +
  Makefile.am |  10 +
  src/sysv-generator/Makefile |   1 +
  src/sysv-generator/sysv-generator.c | 896 
 
  4 files changed, 908 insertions(+)
  create mode 100644 src/sysv-generator/Makefile
  create mode 100644 src/sysv-generator/sysv-generator.c

 diff --git a/.gitignore b/.gitignore
 index 908c563..061b4af 100644
 --- a/.gitignore
 +++ b/.gitignore
 @@ -101,6 +101,7 @@
  /systemd-socket-proxyd
  /systemd-sysctl
  /systemd-system-update-generator
 +/systemd-sysv-generator
  /systemd-timedated
  /systemd-timesyncd
  /systemd-tmpfiles
 diff --git a/Makefile.am b/Makefile.am
 index 5b26bc3..a395969 100644
 --- a/Makefile.am
 +++ b/Makefile.am
 @@ -549,6 +549,7 @@ nodist_systemunit_DATA += \
   units/halt-local.service

  systemgenerator_PROGRAMS += \
 + systemd-sysv-generator \
   systemd-rc-local-generator
  endif

 @@ -1918,6 +1919,15 @@ UNINSTALL_EXEC_HOOKS += dbus1-generator-uninstall-hook
  endif

  # 
 --
 +systemd_sysv_generator_SOURCES = \
 + src/sysv-generator/sysv-generator.c
 +
 +systemd_sysv_generator_LDADD = \
 + libsystemd-core.la \
 + libsystemd-label.la \
 + libsystemd-shared.la
 +
 +# 
 --
  systemd_rc_local_generator_SOURCES = \
   src/rc-local-generator/rc-local-generator.c

 diff --git a/src/sysv-generator/Makefile b/src/sysv-generator/Makefile
 new file mode 100644
 index 000..530e5e9
 --- /dev/null
 +++ b/src/sysv-generator/Makefile
 @@ -0,0 +1 @@
 +../Makefile
 diff --git a/src/sysv-generator/sysv-generator.c 
 b/src/sysv-generator/sysv-generator.c
 new file mode 100644
 index 000..7d153a6
 --- /dev/null
 +++ b/src/sysv-generator/sysv-generator.c
 @@ -0,0 +1,896 @@
 +/*-*- Mode: C; c-basic-offset: 8; indent-tabs-mode: nil -*-*/
 +
 +/***
 +  This file is part of systemd.
 +
 +  Copyright 2014 Thomas H.P. Andersen
 +  Copyright 2010 Lennart Poettering
 +  Copyright 2011 Michal Schmidt
 +
 +  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

Re: [systemd-devel] [PATCH] [RFC] Move handling of sysv initscripts to a generator

[Thomas H.P. Andersen]

On Wed, May 28, 2014 at 1:59 PM, Michael Biebl mbi...@gmail.com wrote:
 2014-05-28 11:58 GMT+02:00 Thomas H.P. Andersen pho...@gmail.com:
 On Wed, May 28, 2014 at 3:38 AM, Zbigniew Jędrzejewski-Szmek
 zbys...@in.waw.pl wrote:
 On Wed, May 28, 2014 at 01:12:23AM +0200, Thomas H.P. Andersen wrote:
 From: Thomas Hindoe Paaboel Andersen pho...@gmail.com

 Reuses logic from service.c and the rc-local generator.

 Note that this drops reading of chkconfig entirely.
 How likely is this to cause regressions in existing distributions?
 The start priority in chkconfig was already removed in 213 so the only
 thing was the runlevels. The runlevels were only used to determine if
 the runlevel was special (not 1-5). Since special runlevels was
 dropped way back I don't see any reason to read the chkconfig at all.

 Wasn't there also the # pidfile: parsing as part of the chkconfig header?
Pidfile is still parsed. I only dropped the line with start priority
and runlevels.

 --
 Why is it that all of the instruments seeking intelligent life in the
 universe are pointed away from Earth?
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [PATCH] [RFC] Move handling of sysv initscripts to a generator

[Thomas H.P. Andersen]

On Wed, May 28, 2014 at 2:14 AM, Peeters Simon peeters.si...@gmail.com wrote:
 2014-05-28 1:12 GMT+02:00 Thomas H.P. Andersen pho...@gmail.com:
 From: Thomas Hindoe Paaboel Andersen pho...@gmail.com

 Reuses logic from service.c and the rc-local generator.

 Note that this drops reading of chkconfig entirely. It also drops reading
 runlevels from the LSB headers. The runlevels were only used to check for
 runlevels outside of the normal 1-5 range and then add special dependencies
 and settings. Special runlevels were dropped in the past so it seemed to be
 unused code.

 Also note that this code behaves differently to the old if an initcsript
 and native unit exist with the same name. Before the initscript would be
 ignored but now a .service file is created in /run which will override
 the native unit.

 this feels wrong, the native unit should override the sysv one.
 Is there any reason why you want to change this?
 Couldn't you just use arg_dest = argv[3]; instead?
Thanks. I have changed it to use this instead.

 The old code dealing with initscripts are left in for now as the generator
 will run first and the old code will then ignore the initscripts since
 unit files exist for them. I will add another patch to rip the old code out
 later.

 The patch is WIP and only tested lightly. I am mostly looking for comments
 if this is a good idea at all.
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

[systemd-devel] [PATCH] [RFC] Move handling of sysv initscripts to a generator

[Thomas H.P. Andersen]

From: Thomas Hindoe Paaboel Andersen pho...@gmail.com

Reuses logic from service.c and the rc-local generator.

Note that this drops reading of chkconfig entirely. It also drops reading
runlevels from the LSB headers. The runlevels were only used to check for
runlevels outside of the normal 1-5 range and then add special dependencies
and settings. Special runlevels were dropped in the past so it seemed to be
unused code.

Also note that this code behaves differently to the old if an initcsript
and native unit exist with the same name. Before the initscript would be
ignored but now a .service file is created in /run which will override
the native unit.

The old code dealing with initscripts are left in for now as the generator
will run first and the old code will then ignore the initscripts since
unit files exist for them. I will add another patch to rip the old code out
later.

The patch is WIP and only tested lightly. I am mostly looking for comments
if this is a good idea at all.
---
 .gitignore  |   1 +
 Makefile.am |  10 +
 src/sysv-generator/Makefile |   1 +
 src/sysv-generator/sysv-generator.c | 896 
 4 files changed, 908 insertions(+)
 create mode 100644 src/sysv-generator/Makefile
 create mode 100644 src/sysv-generator/sysv-generator.c

diff --git a/.gitignore b/.gitignore
index 908c563..061b4af 100644
--- a/.gitignore
+++ b/.gitignore
@@ -101,6 +101,7 @@
 /systemd-socket-proxyd
 /systemd-sysctl
 /systemd-system-update-generator
+/systemd-sysv-generator
 /systemd-timedated
 /systemd-timesyncd
 /systemd-tmpfiles
diff --git a/Makefile.am b/Makefile.am
index 5b26bc3..a395969 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -549,6 +549,7 @@ nodist_systemunit_DATA += \
units/halt-local.service
 
 systemgenerator_PROGRAMS += \
+   systemd-sysv-generator \
systemd-rc-local-generator
 endif
 
@@ -1918,6 +1919,15 @@ UNINSTALL_EXEC_HOOKS += dbus1-generator-uninstall-hook
 endif
 
 # 
--
+systemd_sysv_generator_SOURCES = \
+   src/sysv-generator/sysv-generator.c
+
+systemd_sysv_generator_LDADD = \
+   libsystemd-core.la \
+   libsystemd-label.la \
+   libsystemd-shared.la
+
+# 
--
 systemd_rc_local_generator_SOURCES = \
src/rc-local-generator/rc-local-generator.c
 
diff --git a/src/sysv-generator/Makefile b/src/sysv-generator/Makefile
new file mode 100644
index 000..530e5e9
--- /dev/null
+++ b/src/sysv-generator/Makefile
@@ -0,0 +1 @@
+../Makefile
diff --git a/src/sysv-generator/sysv-generator.c 
b/src/sysv-generator/sysv-generator.c
new file mode 100644
index 000..7d153a6
--- /dev/null
+++ b/src/sysv-generator/sysv-generator.c
@@ -0,0 +1,896 @@
+/*-*- Mode: C; c-basic-offset: 8; indent-tabs-mode: nil -*-*/
+
+/***
+  This file is part of systemd.
+
+  Copyright 2014 Thomas H.P. Andersen
+  Copyright 2010 Lennart Poettering
+  Copyright 2011 Michal Schmidt
+
+  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/.
+***/
+
+#include errno.h
+#include stdio.h
+#include unistd.h
+
+#include util.h
+#include mkdir.h
+#include strv.h
+#include path-util.h
+#include path-lookup.h
+#include log.h
+#include strv.h
+#include unit.h
+#include unit-name.h
+#include special.h
+#include exit-status.h
+#include def.h
+#include env-util.h
+#include fileio.h
+#include hashmap.h
+
+typedef enum RunlevelType {
+RUNLEVEL_UP,
+RUNLEVEL_DOWN
+} RunlevelType;
+
+static const struct {
+const char *path;
+const char *target;
+const RunlevelType type;
+} rcnd_table[] = {
+/* Standard SysV runlevels for start-up */
+{ rc1.d,  SPECIAL_RESCUE_TARGET,RUNLEVEL_UP },
+{ rc2.d,  SPECIAL_RUNLEVEL2_TARGET, RUNLEVEL_UP },
+{ rc3.d,  SPECIAL_RUNLEVEL3_TARGET, RUNLEVEL_UP },
+{ rc4.d,  SPECIAL_RUNLEVEL4_TARGET, RUNLEVEL_UP },
+{ rc5.d,  SPECIAL_RUNLEVEL5_TARGET, RUNLEVEL_UP },
+
+/* Standard SysV runlevels for shutdown */
+{ rc0.d,  SPECIAL_POWEROFF_TARGET,  RUNLEVEL_DOWN },
+{ rc6.d,  SPECIAL_REBOOT_TARGET,RUNLEVEL_DOWN }
+
+/* Note that the order here matters, as we read the
+   directories in this order, and we want to make sure

Re: [systemd-devel] [PATCH] Fix a few compiler warnings

[Thomas H.P. Andersen]

On Tue, May 20, 2014 at 5:43 PM, Lennart Poettering
lenn...@poettering.net wrote:
 On Mon, 19.05.14 19:52, Tom Gundersen (t...@jklm.no) wrote:

   _public_ int sd_peer_get_session(int fd, char **session) {
  -struct ucred ucred;
  +struct ucred ucred = {};

 I can't reproduce this warning, but more importantly, why is this
 necessary in this function and not the subsequent noes (which all seem
 to be more or less equivalent)?

 Hmm, given the current flakiness of the gcc warnings when -flto is in
 the mix I think we should follow the rule that we do not fix gcc
 warnings that show up only with -flto is used. We can revisit that in a
 few years when LTO has settled a bit, but for now I am pretty sure
 trying to fix all those issues is a waste of time and certainly don't
 improve our code...

 Cristian, are those warnings you saw related to -flto?

The warning in namespace_open is not related to LTO. It shows up with
autogen.sh g  make and that gets in the way for my workflow. It
would make my life easier if we could silence it with the fix in this
patch or suppress it with
#pragma GCC diagnostic ignored -Wmaybe-uninitialized

Would that be okay?
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [PATCH] Fix a few compiler warnings

[Thomas H.P. Andersen]

On Wed, May 21, 2014 at 10:46 AM, Lennart Poettering
lenn...@poettering.net wrote:
 On Wed, 21.05.14 10:29, Thomas H.P. Andersen (pho...@gmail.com) wrote:


 On Tue, May 20, 2014 at 5:43 PM, Lennart Poettering
 lenn...@poettering.net wrote:
  On Mon, 19.05.14 19:52, Tom Gundersen (t...@jklm.no) wrote:
 
_public_ int sd_peer_get_session(int fd, char **session) {
   -struct ucred ucred;
   +struct ucred ucred = {};
 
  I can't reproduce this warning, but more importantly, why is this
  necessary in this function and not the subsequent noes (which all seem
  to be more or less equivalent)?
 
  Hmm, given the current flakiness of the gcc warnings when -flto is in
  the mix I think we should follow the rule that we do not fix gcc
  warnings that show up only with -flto is used. We can revisit that in a
  few years when LTO has settled a bit, but for now I am pretty sure
  trying to fix all those issues is a waste of time and certainly don't
  improve our code...
 
  Cristian, are those warnings you saw related to -flto?

 The warning in namespace_open is not related to LTO. It shows up with
 autogen.sh g  make and that gets in the way for my workflow. It
 would make my life easier if we could silence it with the fix in this
 patch or suppress it with
 #pragma GCC diagnostic ignored -Wmaybe-uninitialized

 Would that be okay?

 Ok, fixed that one. It is a false positive, but I can see why gcc gets
 confused, and this sounds OK to fix. Have done so now. Please test!


Works great. Thanks Lennart!
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [PATCH] Fix a few compiler warnings

[Thomas H.P. Andersen]

On Mon, May 19, 2014 at 9:01 PM, Cristian Rodríguez
crrodrig...@opensuse.org wrote:
 El 19/05/14 13:52, Tom Gundersen escribió:

 I can't reproduce this warning, but more importantly, why is this
 necessary in this function and not the subsequent noes (which all
 seem to be more or less equivalent)?

 What compiler did you tried ?  happens with GCC 4.9.0 r209782.

I only see the one for namespace_open and only with -Og. I see it both
on gcc 4.8.2 (f20) and 4.9.0 (rawhide). Did you use any special flags
or something?
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [PATCH 3/3] udev: avoid use of uninitialized err

[Thomas H.P. Andersen]

On Sat, May 17, 2014 at 4:26 AM, Zbigniew Jędrzejewski-Szmek
zbys...@in.waw.pl wrote:
 On Fri, May 16, 2014 at 11:53:25PM +0200, Thomas H.P. Andersen wrote:
 From: Thomas Hindoe Paaboel Andersen pho...@gmail.com

 After 1ea972174baba40dbc80c51cbfc4edc49764b59b err is no longer
 set unless we hit a special case. Initialize it to 0 and remove
 a check that will never fail.
 Looks good too.
Thanks. Applied.

  src/udev/udevd.c | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)

 diff --git a/src/udev/udevd.c b/src/udev/udevd.c
 index bc0696c..1c9488e 100644
 --- a/src/udev/udevd.c
 +++ b/src/udev/udevd.c
 @@ -267,7 +267,7 @@ static void worker_new(struct event *event)
  struct udev_event *udev_event;
  struct worker_message msg;
  int fd_lock = -1;
 -int err;
 +int err = 0;

  log_debug(seq %llu running, 
 udev_device_get_seqnum(dev));
  udev_event = udev_event_new(dev);
 @@ -312,7 +312,7 @@ static void worker_new(struct event *event)
  udev_event_execute_run(udev_event, sigmask_orig);

  /* apply/restore inotify watch */
 -if (err == 0  udev_event-inotify_watch) {
 +if (udev_event-inotify_watch) {
  udev_watch_begin(udev, dev);
  udev_device_update_db(dev);
  }
 --
 1.9.0

 ___
 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

Re: [systemd-devel] [PATCH 2/3] systemctl: more cleanup

[Thomas H.P. Andersen]

On Sat, May 17, 2014 at 4:22 AM, Zbigniew Jędrzejewski-Szmek
zbys...@in.waw.pl wrote:
 On Fri, May 16, 2014 at 11:53:24PM +0200, Thomas H.P. Andersen wrote:
 From: Thomas Hindoe Paaboel Andersen pho...@gmail.com

 This is followup on 05cae7f3431446236139434ee58a6275f3cb31e8

 I think the intention was to use the newly introduced 'path'
 variable in the inner loop instead of p.
 Thanks for catching this. Looks good.
Thanks. Applied.

  src/systemctl/systemctl.c | 8 
  1 file changed, 4 insertions(+), 4 deletions(-)

 diff --git a/src/systemctl/systemctl.c b/src/systemctl/systemctl.c
 index 2fa8ecc..6e98c05 100644
 --- a/src/systemctl/systemctl.c
 +++ b/src/systemctl/systemctl.c
 @@ -5030,16 +5030,16 @@ static int enable_sysv_units(const char *verb, char 
 **args) {
  _cleanup_free_ char *path = NULL;

  if (!isempty(arg_root))
 -asprintf(p, %s/%s/%s, arg_root, *k, 
 name);
 +asprintf(path, %s/%s/%s, arg_root, *k, 
 name);
  else
 -asprintf(p, %s/%s, *k, name);
 +asprintf(path, %s/%s, *k, name);

 -if (!p) {
 +if (!path) {
  r = log_oom();
  goto finish;
  }

 -found_native = access(p, F_OK) = 0;
 +found_native = access(path, F_OK) = 0;
  if (found_native)
  break;
  }
 --
 1.9.0

 ___
 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

Re: [systemd-devel] [PATCH 1/3] sd-bus: avoid potentially unrefing a unitialized sd_bus_slot

[Thomas H.P. Andersen]

On Sat, May 17, 2014 at 12:07 AM, Zbigniew Jędrzejewski-Szmek
zbys...@in.waw.pl wrote:
 On Fri, May 16, 2014 at 11:53:23PM +0200, Thomas H.P. Andersen wrote:
 From: Thomas Hindoe Paaboel Andersen pho...@gmail.com

 ---
  src/libsystemd/sd-bus/bus-objects.c | 2 +-
  src/libsystemd/sd-bus/sd-bus.c  | 2 +-
  2 files changed, 2 insertions(+), 2 deletions(-)

 diff --git a/src/libsystemd/sd-bus/bus-objects.c 
 b/src/libsystemd/sd-bus/bus-objects.c
 index 51d4a62..dbb04e5 100644
 --- a/src/libsystemd/sd-bus/bus-objects.c
 +++ b/src/libsystemd/sd-bus/bus-objects.c
 @@ -1602,7 +1602,7 @@ static int add_object_vtable_internal(
  sd_bus_object_find_t find,
  void *userdata) {

 -sd_bus_slot *s;
 +sd_bus_slot *s = NULL;
  struct node_vtable *i, *existing = NULL;
  const sd_bus_vtable *v;
  struct node *n;
 diff --git a/src/libsystemd/sd-bus/sd-bus.c b/src/libsystemd/sd-bus/sd-bus.c
 index ec2843f..1f1a4d3 100644
 --- a/src/libsystemd/sd-bus/sd-bus.c
 +++ b/src/libsystemd/sd-bus/sd-bus.c
 @@ -2821,7 +2821,7 @@ _public_ int sd_bus_add_match(

  struct bus_match_component *components = NULL;
  unsigned n_components = 0;
 -sd_bus_slot *s;
 +sd_bus_slot *s = NULL;
  int r = 0;

  assert_return(bus, -EINVAL);
 That makes it the third identical patch :)

Heh. At least we agree on the fix :)
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

[systemd-devel] [PATCH 2/3] systemctl: more cleanup

[Thomas H.P. Andersen]

From: Thomas Hindoe Paaboel Andersen pho...@gmail.com

This is followup on 05cae7f3431446236139434ee58a6275f3cb31e8

I think the intention was to use the newly introduced 'path'
variable in the inner loop instead of p.
---
 src/systemctl/systemctl.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/src/systemctl/systemctl.c b/src/systemctl/systemctl.c
index 2fa8ecc..6e98c05 100644
--- a/src/systemctl/systemctl.c
+++ b/src/systemctl/systemctl.c
@@ -5030,16 +5030,16 @@ static int enable_sysv_units(const char *verb, char 
**args) {
 _cleanup_free_ char *path = NULL;
 
 if (!isempty(arg_root))
-asprintf(p, %s/%s/%s, arg_root, *k, name);
+asprintf(path, %s/%s/%s, arg_root, *k, 
name);
 else
-asprintf(p, %s/%s, *k, name);
+asprintf(path, %s/%s, *k, name);
 
-if (!p) {
+if (!path) {
 r = log_oom();
 goto finish;
 }
 
-found_native = access(p, F_OK) = 0;
+found_native = access(path, F_OK) = 0;
 if (found_native)
 break;
 }
-- 
1.9.0

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

[systemd-devel] [PATCH 3/3] udev: avoid use of uninitialized err

[Thomas H.P. Andersen]

From: Thomas Hindoe Paaboel Andersen pho...@gmail.com

After 1ea972174baba40dbc80c51cbfc4edc49764b59b err is no longer
set unless we hit a special case. Initialize it to 0 and remove
a check that will never fail.
---
 src/udev/udevd.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/udev/udevd.c b/src/udev/udevd.c
index bc0696c..1c9488e 100644
--- a/src/udev/udevd.c
+++ b/src/udev/udevd.c
@@ -267,7 +267,7 @@ static void worker_new(struct event *event)
 struct udev_event *udev_event;
 struct worker_message msg;
 int fd_lock = -1;
-int err;
+int err = 0;
 
 log_debug(seq %llu running, 
udev_device_get_seqnum(dev));
 udev_event = udev_event_new(dev);
@@ -312,7 +312,7 @@ static void worker_new(struct event *event)
 udev_event_execute_run(udev_event, sigmask_orig);
 
 /* apply/restore inotify watch */
-if (err == 0  udev_event-inotify_watch) {
+if (udev_event-inotify_watch) {
 udev_watch_begin(udev, dev);
 udev_device_update_db(dev);
 }
-- 
1.9.0

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

[systemd-devel] [PATCH 1/3] sd-bus: avoid potentially unrefing a unitialized sd_bus_slot

[Thomas H.P. Andersen]

From: Thomas Hindoe Paaboel Andersen pho...@gmail.com

---
 src/libsystemd/sd-bus/bus-objects.c | 2 +-
 src/libsystemd/sd-bus/sd-bus.c  | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/libsystemd/sd-bus/bus-objects.c 
b/src/libsystemd/sd-bus/bus-objects.c
index 51d4a62..dbb04e5 100644
--- a/src/libsystemd/sd-bus/bus-objects.c
+++ b/src/libsystemd/sd-bus/bus-objects.c
@@ -1602,7 +1602,7 @@ static int add_object_vtable_internal(
 sd_bus_object_find_t find,
 void *userdata) {
 
-sd_bus_slot *s;
+sd_bus_slot *s = NULL;
 struct node_vtable *i, *existing = NULL;
 const sd_bus_vtable *v;
 struct node *n;
diff --git a/src/libsystemd/sd-bus/sd-bus.c b/src/libsystemd/sd-bus/sd-bus.c
index ec2843f..1f1a4d3 100644
--- a/src/libsystemd/sd-bus/sd-bus.c
+++ b/src/libsystemd/sd-bus/sd-bus.c
@@ -2821,7 +2821,7 @@ _public_ int sd_bus_add_match(
 
 struct bus_match_component *components = NULL;
 unsigned n_components = 0;
-sd_bus_slot *s;
+sd_bus_slot *s = NULL;
 int r = 0;
 
 assert_return(bus, -EINVAL);
-- 
1.9.0

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

[systemd-devel] [PATCH] silence warning

[Thomas H.P. Andersen]

From: Thomas Hindoe Paaboel Andersen pho...@gmail.com

The error message logged in finish: will not be very informative
but on the other hand I don't see how this should ever happen.
---
 src/network/networkd-link.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/src/network/networkd-link.c b/src/network/networkd-link.c
index 684e1e5..e9b30a3 100644
--- a/src/network/networkd-link.c
+++ b/src/network/networkd-link.c
@@ -1518,8 +1518,10 @@ int link_save(Link *link) {
 assert(link-state_file);
 
 state = link_state_to_string(link-state);
-if (!state)
+if (!state) {
+r = -EINVAL;
 goto finish;
+}
 
 r = fopen_temporary(link-state_file, f, temp_path);
 if (r  0)
-- 
1.9.0

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

Re: [systemd-devel] [PATCH] journal-remote-parse: avoid passing null to memchr

[Thomas H.P. Andersen]

On Sat, Apr 5, 2014 at 10:08 PM, Zbigniew Jędrzejewski-Szmek
zbys...@in.waw.pl wrote:
 On Sat, Apr 05, 2014 at 09:09:47PM +0200, Thomas H.P. Andersen wrote:
  static int get_line(RemoteSource *source, char **line, size_t *size) {
  ssize_t n, remain;
 -char *c;
 +char *c = NULL;
  char *newbuf = NULL;
  size_t newsize = 0;

 @@ -49,7 +49,9 @@ static int get_line(RemoteSource *source, char **line, 
 size_t *size) {
  assert(source-filled = source-size);
  assert(source-buf == NULL || source-size  0);

 -c = memchr(source-buf, '\n', source-filled);
 +if (source-buf)
 +c = memchr(source-buf, '\n', source-filled);
 +
  if (c != NULL)
 Looks good. Please push.

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

[systemd-devel] [PATCH] journal-remote-parse: avoid passing null to memchr

[Thomas H.P. Andersen]

From: Thomas Hindoe Paaboel Andersen pho...@gmail.com

Found with scan-build
---
 src/journal/journal-remote-parse.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/src/journal/journal-remote-parse.c 
b/src/journal/journal-remote-parse.c
index 142de0e..239ff38 100644
--- a/src/journal/journal-remote-parse.c
+++ b/src/journal/journal-remote-parse.c
@@ -40,7 +40,7 @@ void source_free(RemoteSource *source) {
 
 static int get_line(RemoteSource *source, char **line, size_t *size) {
 ssize_t n, remain;
-char *c;
+char *c = NULL;
 char *newbuf = NULL;
 size_t newsize = 0;
 
@@ -49,7 +49,9 @@ static int get_line(RemoteSource *source, char **line, size_t 
*size) {
 assert(source-filled = source-size);
 assert(source-buf == NULL || source-size  0);
 
-c = memchr(source-buf, '\n', source-filled);
+if (source-buf)
+c = memchr(source-buf, '\n', source-filled);
+
 if (c != NULL)
 goto docopy;
 
-- 
1.9.0

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

Re: [systemd-devel] [PATCH] build-sys: workaround scan-build bug to fix ./autogen.sh s

[Thomas H.P. Andersen]

On Tue, Apr 1, 2014 at 3:00 PM, Daniel Buch boogiewasth...@gmail.com wrote:
 It seems to be a clang-analyzer problem since it don't behave like
 clang-compiler regarding -std={c99,gnu99}
 ---
  autogen.sh | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

 diff --git a/autogen.sh b/autogen.sh
 index 9b4781c..449a2e6 100755
 --- a/autogen.sh
 +++ b/autogen.sh
 @@ -72,7 +72,7 @@ elif [ x$1 = xl ]; then
  $topdir/configure CC=clang CFLAGS='-g -O0 -ftrapv -Wno-gnu' 
 --enable-compat-libs --enable-kdbus $args
  make clean
  elif [ x$1 = xs ]; then
 -scan-build $topdir/configure CFLAGS='-g -O0 -ftrapv' 
 --enable-compat-libs --enable-kdbus $args
 +scan-build $topdir/configure CFLAGS='-std=gnu99 -g -O0 -ftrapv' 
 --enable-compat-libs --enable-kdbus $args
  scan-build make
  else
  echo
 --
 1.9.1

Applied. This also fixed the same problem with clang 3.4 on rawhide.

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

Re: [systemd-devel] [PATCH] systemd-run: extend bash completion

[Thomas H.P. Andersen]

On Tue, Mar 11, 2014 at 2:29 PM, Lennart Poettering
lenn...@poettering.net wrote:
 On Tue, 11.03.14 08:05, Thomas H.P. Andersen (pho...@gmail.com) wrote:


 On Tue, Mar 11, 2014 at 4:52 AM, Lennart Poettering
 lenn...@poettering.net wrote:
  On Wed, 05.03.14 23:48, Thomas H.P. Andersen (pho...@gmail.com) wrote:
 
  From: Thomas Hindoe Paaboel Andersen pho...@gmail.com
 
  --system
  -H --host
  -M --machine
  --service-type (options: simple forking oneshot dbus notify idle)
  --uid
  --gid
  --nice
  --setenv
  -p --property (options read from bus_append_unit_property_assignment)
 
  Looks good as far as I grok the completion things. Zbigniew, you know
  this better than I do, can you OK this?

 I was mostly looking for a verification that the list of options make
 are correct:

 --service-type:
 simple forking oneshot dbus notify idle

 Corrct.

 --property:
 CPUAccounting MemoryAccounting BlockIOAccounting SendSIGHUP
 SendSIGKILL MemoryLimit CPUShares BlockIOWeight User Group
 DevicePolicy KillMode DeviceAllow BlockIOReadBandwidth
 BlockIOWriteBandwidth BlockIODeviceWeight Nice Environment KillSignal

 Some of the properties take e.g. a boolean value and we could go on to
 suggest an on/off value but I did not go that far.

 Given that there is --user= and --group= it's a bit pointless to ever
 specify User= and Group= with --property, but then again, it will work.

 There are also the LimitCORE=, LimitCPU=, ... props settable now.


Pushed with the extra LimitXYZ's
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [PATCH] systemd-run: extend bash completion

[Thomas H.P. Andersen]

On Tue, Mar 11, 2014 at 4:52 AM, Lennart Poettering
lenn...@poettering.net wrote:
 On Wed, 05.03.14 23:48, Thomas H.P. Andersen (pho...@gmail.com) wrote:

 From: Thomas Hindoe Paaboel Andersen pho...@gmail.com

 --system
 -H --host
 -M --machine
 --service-type (options: simple forking oneshot dbus notify idle)
 --uid
 --gid
 --nice
 --setenv
 -p --property (options read from bus_append_unit_property_assignment)

 Looks good as far as I grok the completion things. Zbigniew, you know
 this better than I do, can you OK this?

I was mostly looking for a verification that the list of options make
are correct:

--service-type:
simple forking oneshot dbus notify idle

--property:
CPUAccounting MemoryAccounting BlockIOAccounting SendSIGHUP
SendSIGKILL MemoryLimit CPUShares BlockIOWeight User Group
DevicePolicy KillMode DeviceAllow BlockIOReadBandwidth
BlockIOWriteBandwidth BlockIODeviceWeight Nice Environment KillSignal

Some of the properties take e.g. a boolean value and we could go on to
suggest an on/off value but I did not go that far.
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

[systemd-devel] [PATCH] systemd-run: extend bash completion

[Thomas H.P. Andersen]

From: Thomas Hindoe Paaboel Andersen pho...@gmail.com

--system
-H --host
-M --machine
--service-type (options: simple forking oneshot dbus notify idle)
--uid
--gid
--nice
--setenv
-p --property (options read from bus_append_unit_property_assignment)
---
 shell-completion/bash/systemd-run | 39 +--
 1 file changed, 37 insertions(+), 2 deletions(-)

diff --git a/shell-completion/bash/systemd-run 
b/shell-completion/bash/systemd-run
index ab55274..d4e4f98 100644
--- a/shell-completion/bash/systemd-run
+++ b/shell-completion/bash/systemd-run
@@ -25,9 +25,16 @@ __systemctl() {
 __get_slice_units () { __systemctl $1 list-units --all -t slice \
 | { while read -r a b c d; do echo  $a; done; }; }
 
+__get_machines() {
+local a b
+machinectl list --no-legend --no-pager | { while read a b; do echo  
$a; done; };
+}
+
 _systemd_run() {
 local cur=${COMP_WORDS[COMP_CWORD]} prev=${COMP_WORDS[COMP_CWORD-1]}
-local OPTS='-h --help --version --user --scope --unit --description 
--slice -r --remain-after-exit --send-sighup'
+local OPTS='-h --help --version --user --system --scope --unit 
--description --slice
+-r --remain-after-exit --send-sighup -H --host -M --machine 
--service-type
+--uid --gid --nice --setenv -p --property'
 
 local mode=--system
 local i
@@ -40,7 +47,7 @@ _systemd_run() {
 
 [[ ${COMP_WORDS[i]} == --user ]]  mode=--user
 
-[[ $i -lt $COMP_CWORD  ${COMP_WORDS[i]} == 
@(--unit|--description|--slice) ]]  ((i++))
+[[ $i -lt $COMP_CWORD  ${COMP_WORDS[i]} == 
@(--unit|--description|--slice|--service-type|-H|--host|-M|--machine|-p|--property)
 ]]  ((i++))
 done
 
 case $prev in
@@ -54,6 +61,34 @@ _systemd_run() {
 COMPREPLY=( $(compgen -W '$comps' -- $cur) )
 return 0
 ;;
+--service-type)
+local comps='simple forking oneshot dbus notify idle'
+
+COMPREPLY=( $(compgen -W '$comps' -- $cur) )
+return 0
+;;
+-p|--property)
+local comps='CPUAccounting= MemoryAccounting= BlockIOAccounting= 
SendSIGHUP=
+ SendSIGKILL= MemoryLimit= CPUShares= BlockIOWeight= 
User= Group=
+ DevicePolicy= KillMode= DeviceAllow= 
BlockIOReadBandwidth=
+ BlockIOWriteBandwidth= BlockIODeviceWeight= Nice= 
Environment=
+ KillSignal='
+
+COMPREPLY=( $(compgen -W '$comps' -- $cur) )
+return 0
+;;
+-H|--host)
+local comps=$(compgen -A hostname)
+
+COMPREPLY=( $(compgen -W '$comps' -- $cur) )
+return 0
+;;
+-M|--machine)
+local comps=$( __get_machines )
+
+COMPREPLY=( $(compgen -W '$comps' -- $cur) )
+return 0
+;;
 esac
 
 COMPREPLY=( $(compgen -W '${OPTS[*]}' -- $cur) )
-- 
1.8.5.3

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

[systemd-devel] [PATCH] networkd-wait-online: use automatic cleanup

[Thomas H.P. Andersen]

From: Thomas Hindoe Paaboel Andersen pho...@gmail.com

---
Only compile tested. I just wanted to get rid of the warnings about
use of the uninitialized variables.

 Makefile.am|  3 ++-
 src/network/network-util.h | 28 
 src/network/networkd-wait-online.c | 12 +---
 3 files changed, 35 insertions(+), 8 deletions(-)
 create mode 100644 src/network/network-util.h

diff --git a/Makefile.am b/Makefile.am
index c4a975d..1ad2756 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -2417,7 +2417,8 @@ libsystemd_network_la_SOURCES = \
src/libsystemd-network/dhcp-internal.h \
src/libsystemd-network/dhcp-protocol.h \
src/libsystemd-network/dhcp-lease-internal.h \
-   src/libsystemd-network/sd-dhcp-lease.c
+   src/libsystemd-network/sd-dhcp-lease.c \
+   src/libsystemd-network/network-util.h
 
 libsystemd_network_la_LIBADD = \
libsystemd-internal.la \
diff --git a/src/network/network-util.h b/src/network/network-util.h
new file mode 100644
index 000..9979e6d
--- /dev/null
+++ b/src/network/network-util.h
@@ -0,0 +1,28 @@
+/*-*- Mode: C; c-basic-offset: 8; indent-tabs-mode: nil -*-*/
+
+#pragma once
+
+/***
+  This file is part of systemd.
+
+  Copyright 2014 Thomas Hindø Paabøl Andersen
+
+  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/.
+***/
+
+#include util.h
+
+DEFINE_TRIVIAL_CLEANUP_FUNC(sd_network_monitor*, sd_network_monitor_unref);
+
+#define _cleanup_network_monitor_unref_ _cleanup_(sd_network_monitor_unrefp)
diff --git a/src/network/networkd-wait-online.c 
b/src/network/networkd-wait-online.c
index 51c6bbd..50b753b 100644
--- a/src/network/networkd-wait-online.c
+++ b/src/network/networkd-wait-online.c
@@ -20,8 +20,10 @@
 ***/
 
 #include sd-event.h
+#include event-util.h
 #include sd-daemon.h
 #include sd-network.h
+#include network-util.h
 
 #include util.h
 
@@ -62,9 +64,9 @@ static int event_handler(sd_event_source *s, int fd, uint32_t 
revents,
 }
 
 int main(int argc, char *argv[]) {
-sd_event *event;
-sd_event_source *event_source;
-sd_network_monitor *monitor;
+_cleanup_event_unref_ sd_event *event = NULL;
+_cleanup_event_source_unref_ sd_event_source *event_source = NULL;
+_cleanup_network_monitor_unref_ sd_network_monitor *monitor = NULL;
 int r, fd, events;
 
 log_set_target(LOG_TARGET_AUTO);
@@ -129,9 +131,5 @@ out:
 sd_notify(false,
   STATUS=All interfaces configured...);
 
-sd_event_source_unref(event_source);
-sd_event_unref(event);
-sd_network_monitor_unref(monitor);
-
 return r  0 ? EXIT_FAILURE : EXIT_SUCCESS;
 }
-- 
1.8.5.3

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

[systemd-devel] pcre in daemons

[Thomas H.P. Andersen]

The todo says:
something pulls in pcre as shared object dependency into our daemons
such as hostnamed

Normal buiild:
ldd ./systemd-hostnamed
linux-vdso.so.1 =  (0x7fff247bc000)
libselinux.so.1 = /lib64/libselinux.so.1 (0x7f7ec47f7000)
librt.so.1 = /lib64/librt.so.1 (0x7f7ec45ef000)
libdl.so.2 = /lib64/libdl.so.2 (0x7f7ec43ea000)
libpthread.so.0 = /lib64/libpthread.so.0 (0x7f7ec41cd000)
libc.so.6 = /lib64/libc.so.6 (0x7f7ec3e0e000)
/lib64/ld-linux-x86-64.so.2 (0x7f7ec4a2f000)
libpcre.so.1 = /lib64/libpcre.so.1 (0x7f7ec3ba7000)
liblzma.so.5 = /lib64/liblzma.so.5 (0x7f7ec3982000)


With --disable-selinux:
$ ldd ./systemd-hostnamed
linux-vdso.so.1 =  (0x7fff1a651000)
librt.so.1 = /lib64/librt.so.1 (0x7f1c058d)
libdl.so.2 = /lib64/libdl.so.2 (0x7f1c056cc000)
libpthread.so.0 = /lib64/libpthread.so.0 (0x7f1c054ae000)
libc.so.6 = /lib64/libc.so.6 (0x7f1c050ef000)
/lib64/ld-linux-x86-64.so.2 (0x7f1c05aec000)

Not sure what to do about that though...
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

[systemd-devel] [PATCH] detect_virtualization: make Virtualization an out value

[Thomas H.P. Andersen]

From: Thomas Hindoe Paaboel Andersen pho...@gmail.com

The return value from detect_virtualization used to be a
Virtualization enum but in cases of error it would also be a
negative errno. This caused a warning in clang when test-architecture
began comparing the return value to -EPERM and -EACCES.
---
 src/core/dbus-manager.c   |  2 +-
 src/core/main.c   |  2 +-
 src/detect-virt/detect-virt.c |  6 +++---
 src/hostname/hostnamectl.c|  3 ++-
 src/hostname/hostnamed.c  |  2 +-
 src/shared/condition-util.c   |  8 
 src/shared/virt.c | 20 ++--
 src/shared/virt.h |  2 +-
 src/test/test-architecture.c  |  5 +++--
 9 files changed, 30 insertions(+), 20 deletions(-)

diff --git a/src/core/dbus-manager.c b/src/core/dbus-manager.c
index 75004cb..f5a06ba 100644
--- a/src/core/dbus-manager.c
+++ b/src/core/dbus-manager.c
@@ -84,7 +84,7 @@ static int property_get_virtualization(
 assert(bus);
 assert(reply);
 
-detect_virtualization(id);
+detect_virtualization(id, NULL);
 
 return sd_bus_message_append(reply, s, id);
 }
diff --git a/src/core/main.c b/src/core/main.c
index 086e283..5facd17 100644
--- a/src/core/main.c
+++ b/src/core/main.c
@@ -1507,7 +1507,7 @@ int main(int argc, char *argv[]) {
 
 log_info(PACKAGE_STRING  running in system mode. ( 
SYSTEMD_FEATURES ));
 
-detect_virtualization(virtualization);
+detect_virtualization(virtualization, NULL);
 if (virtualization)
 log_info(Detected virtualization '%s'., 
virtualization);
 
diff --git a/src/detect-virt/detect-virt.c b/src/detect-virt/detect-virt.c
index 2f8b0eb..698658c 100644
--- a/src/detect-virt/detect-virt.c
+++ b/src/detect-virt/detect-virt.c
@@ -131,9 +131,9 @@ int main(int argc, char *argv[]) {
 case ANY_VIRTUALIZATION: {
 Virtualization v;
 
-v = detect_virtualization(id);
-if (v  0) {
-log_error(Failed to check for virtualization: %s, 
strerror(-v));
+r = detect_virtualization(id, v);
+if (r  0) {
+log_error(Failed to check for virtualization: %s, 
strerror(-r));
 return EXIT_FAILURE;
 }
 
diff --git a/src/hostname/hostnamectl.c b/src/hostname/hostnamectl.c
index e455249..2e1e6d7 100644
--- a/src/hostname/hostnamectl.c
+++ b/src/hostname/hostnamectl.c
@@ -104,7 +104,8 @@ static void print_status_info(StatusInfo *i) {
 if (r = 0)
 printf(   Boot ID:  SD_ID128_FORMAT_STR \n, 
SD_ID128_FORMAT_VAL(bid));
 
-if (detect_virtualization(id)  0)
+r = detect_virtualization(id, NULL);
+if (r  0)
 printf(Virtualization: %s\n, id);
 
 r = parse_env_file(/etc/os-release, NEWLINE,
diff --git a/src/hostname/hostnamed.c b/src/hostname/hostnamed.c
index e57891b..2e36d01 100644
--- a/src/hostname/hostnamed.c
+++ b/src/hostname/hostnamed.c
@@ -125,7 +125,7 @@ static const char* fallback_chassis(void) {
 unsigned t;
 Virtualization v;
 
-v = detect_virtualization(NULL);
+r = detect_virtualization(NULL, v);
 
 if (v == VIRTUALIZATION_VM)
 return vm;
diff --git a/src/shared/condition-util.c b/src/shared/condition-util.c
index 4aea3ca..44feabe 100644
--- a/src/shared/condition-util.c
+++ b/src/shared/condition-util.c
@@ -121,7 +121,7 @@ bool condition_test_kernel_command_line(Condition *c) {
 }
 
 bool condition_test_virtualization(Condition *c) {
-int b;
+int b, r;
 Virtualization v;
 const char *id;
 
@@ -129,9 +129,9 @@ bool condition_test_virtualization(Condition *c) {
 assert(c-parameter);
 assert(c-type == CONDITION_VIRTUALIZATION);
 
-v = detect_virtualization(id);
-if (v  0) {
-log_warning(Failed to detect virtualization, ignoring: %s, 
strerror(-v));
+r = detect_virtualization(id, v);
+if (r  0) {
+log_warning(Failed to detect virtualization, ignoring: %s, 
strerror(-r));
 return c-negate;
 }
 
diff --git a/src/shared/virt.c b/src/shared/virt.c
index c79d35d..73ce9ff 100644
--- a/src/shared/virt.c
+++ b/src/shared/virt.c
@@ -278,20 +278,28 @@ finish:
 }
 
 /* Returns a short identifier for the various VM/container implementations */
-Virtualization detect_virtualization(const char **id) {
+int detect_virtualization(const char **id, Virtualization *type) {
 int r;
 
 r = detect_container(id);
 if (r  0)
 return r;
-if (r  0)
-return VIRTUALIZATION_CONTAINER;
+if (r  0) {
+if (type != NULL)
+*type = VIRTUALIZATION_CONTAINER;
+return r;
+}
 
 r = detect_vm(id);
 if (r  0)
  

[systemd-devel] [PATCH] man: networkd typo fixes

[Thomas H.P. Andersen]

From: Thomas Hindoe Paaboel Andersen pho...@gmail.com

---
Not too sure about the bridge - netdev change. It just seemed a strange to read
since we set the type a bit later to be either bridge, bond, or vlan.

 man/systemd-networkd.service.xml | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/man/systemd-networkd.service.xml b/man/systemd-networkd.service.xml
index 51ae761..026536e 100644
--- a/man/systemd-networkd.service.xml
+++ b/man/systemd-networkd.service.xml
@@ -68,7 +68,7 @@
 
 paraNetwork configurations applied before networkd is started
 are not removed, and static configuration applied by networkd
-are not removed when networkd exits. This ensures restarting
+is not removed when networkd exits. This ensures restarting
 networkd does not cut the network connection, and, in 
particular,
 that it is safe to transition between the initrd and the real 
root,
 and back./para
@@ -153,7 +153,7 @@
 termvarnameName=/varname/term
 listitem
 paraThe interface name used 
when creating the
-bridge. This option is 
compulsory./para
+netdev. This option is 
compulsory./para
 /listitem
 /varlistentry
 varlistentry
-- 
1.8.5.3

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