[systemd-devel] Need advice on daemon's architecture
Hello All! I'm working on a system service which uses systemd intensively. Right now it's socket-activated, with main service of type simple. I recently added support for querying and publishing some internals via D-Bus, so it has a D-Bus name now. Does it add anything if I change type of a main service to dbus thus allowing systemd to know for sure if my service is fully initialized? -- With best regards, Peter Lemenkov. ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] Need advice on daemon's architecture
On Sat, Nov 2, 2013 at 11:40 PM, Peter Lemenkov lemen...@gmail.com wrote: Does it add anything if I change type of a main service to dbus thus allowing systemd to know for sure if my service is fully initialized? Yes. Changing to Type=dbus will cause systemd to only consider the service fully started after there's a listener for whatever's specified with BusName= [1]. [1] http://www.freedesktop.org/software/systemd/man/systemd.service.html#Type= ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH 1/2]add parameters checking for 'udevadm settle'
-Original Message-
From: Zbigniew Jędrzejewski-Szmek [mailto:zbys...@in.waw.pl]
Sent: Tuesday, October 29, 2013 9:38 PM
To: YangZhiyong
Cc: systemd-devel@lists.freedesktop.org; k...@vrfy.org
Subject: Re: [systemd-devel] [PATCH 1/2]add parameters checking for
'udevadm settle'
On Tue, Oct 29, 2013 at 01:34:33PM +0800, YangZhiyong wrote:
-Original Message-
From: YangZhiyong [mailto:yangzy.f...@cn.fujitsu.com]
Sent: Tuesday, October 29, 2013 11:33 AM
To: 'Zbigniew Jędrzejewski-Szmek'
Cc: systemd-devel@lists.freedesktop.org; k...@vrfy.org; fnst-
dri...@cn.fujitsu.com
Subject: 答复: [systemd-devel] [PATCH 1/2]add parameters checking for
'udevadm settle'
- Original Message -
From: Zbigniew Jędrzejewski-Szmek [mailto:zbys...@in.waw.pl]
Sent: October 27, 2013 3:29 PM
To: Yang Zhiyong
Cc: systemd-devel@lists.freedesktop.org; k...@vrfy.org
Subject: Re: [systemd-devel] [PATCH 1/2]add parameters checking
for 'udevadm settle'
On Wed, Oct 23, 2013 at 03:09:36PM +0800, Yang Zhiyong wrote:
This patch adds parameters checking for 'udevadm settle'
Signed-off-by: Yang Zhiyong yangzy.f...@cn.fujitsu.com
---
src/udev/udevadm-settle.c | 15 ---
1 files changed, 12 insertions(+), 3 deletions(-) mode change
100644 = 100755 src/udev/udevadm-settle.c
diff --git a/src/udev/udevadm-settle.c
b/src/udev/udevadm-settle.c old mode 100644 new mode 100755
index c4fc4ee..cacc7e3
--- a/src/udev/udevadm-settle.c
+++ b/src/udev/udevadm-settle.c
@@ -62,8 +62,13 @@ static int adm_settle(struct udev *udev, int
argc, char
*argv[])
int seconds;
option = getopt_long(argc, argv, s:e:t:E:qh,
options,
NULL);
-if (option == -1)
+if (option == -1) {
+if (optind argc) {
+fprintf(stderr, unknown option\n);
+exit(EXIT_FAILURE);
+}
break;
+}
I don't think that getopt_long will return -1 for an unknown option.
According to the manpage, -1 mean that all command-line options
have
been parsed.
I'm very sure that getopt_long() will return -1 when we use an
unknown option such as udevadm settle bjjkhgjk by using
printf(option=%d\n,option); in the code.
If we use udevadm settle --bjjkhgjk(add --)the that getopt_long()
will
return ? as the manpage said, and print error info.
But if lacking--, getopt_long() will treat bjjkhgjk as parameter
instead
of option .
Because of this , getopt_long() returns -1 and does not print any error
info.
By the way,some other source code also uses the same method to
process the more rigorous parameter checking.
Yes! So your code is fine, but please don't say unknown option, but rather
Extraneous argument: %s or something like that.
switch (option) {
case 's':
@@ -74,10 +79,14 @@ static int adm_settle(struct udev *udev, int
argc,
char *argv[])
break;
case 't':
seconds = atoi(optarg);
-if (seconds = 0)
+if (seconds 0)
timeout = seconds;
-else
+else if (seconds == 0 !strcmp(optarg,0))
+timeout = seconds;
+else {
fprintf(stderr, invalid
timeout value\n);
+exit(EXIT_FAILURE);
+}
break;
case 'q':
quiet = 1;
This looks legitimate, but please use safe_atou().
You mean using safe_atoi() ?
No, atou, because the timeout must be non-negative.
Because the type of seconds is int.
If we use atou,GCC will print WARNING ./src/shared/util.h:136:5: note:
expected ‘unsigned int *’ but argument is of type ‘int *’
because the function prototype of safe_atoi is int safe_atou(const char *s,
unsigned *ret_u); .
Shall I write like this:
case 't':
if (safe_atoi(optarg,seconds) 0){
fprintf(stderr, invalid timeout value\n);
exit(EXIT_FAILURE);
}
break;
The error message can be improved:
int r;
r = safe_atou(optarg, seconds);
if (r 0) {
I think I should modify 2 lines of code above like this:
r = safe_atoi(optarg,seconds);
if (r 0 || seconds 0) {
And I have test it successfully.
Yang Zhiyong
fprintf(stderr, Invalid timeout value
Re: [systemd-devel] tree-wide conversion from libdbus to libsystemd-bus
On 10/30/2013 03:48 AM, Kay Sievers wrote: On Wed, Oct 23, 2013 at 1:22 AM, Kay Sievers k...@vrfy.org wrote: [update] To avoid any duplication of work, here are the tools which still need conversion. Please reply to this mail, in case you decide to work on anything in that area. - timedatectl - systemd-logind - loginctl Peeters Simon: I'll take ... (probably loginctl afterwards) - localectl Kay will do that next - hostnamectl - pam_systemd Zbigniew: I'll do pam_systemd - systemctl I'll have a look at systemctl. Seems like a good way to get familiar with the new API. Might take me some days to finish it though. Daniel ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] Need advice on daemon's architecture
'Twas brillig, and Peter Lemenkov at 03/11/13 06:40 did gyre and gimble: Hello All! I'm working on a system service which uses systemd intensively. Right now it's socket-activated, with main service of type simple. I recently added support for querying and publishing some internals via D-Bus, so it has a D-Bus name now. Does it add anything if I change type of a main service to dbus thus allowing systemd to know for sure if my service is fully initialized? If you are using systemd intensively, then you may want to use Type=notify. With type=dbus, systemd will consider things ready when you take the name on the bus, but this might not actually be the last thing you do to initialise your daemon (although if this is the only interface for clients this is not a problem!). You still might want to use sd_notify() instead. This can also pass through some internal performance info to systemd which will show up on systemctl status output which is kinda nice. Col -- Colin Guthrie gmane(at)colin.guthr.ie http://colin.guthr.ie/ Day Job: Tribalogic Limited http://www.tribalogic.net/ Open Source: Mageia Contributor http://www.mageia.org/ PulseAudio Hacker http://www.pulseaudio.org/ Trac Hacker http://trac.edgewall.org/ ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH 1/2] add parameters checking for 'udevadm settle
This patch adds parameters checking for 'udevadm settle'.
---
src/udev/udevadm-settle.c | 20 ++--
1 file changed, 14 insertions(+), 6 deletions(-)
diff --git a/src/udev/udevadm-settle.c b/src/udev/udevadm-settle.c
index c4fc4ee..06f4004 100644
--- a/src/udev/udevadm-settle.c
+++ b/src/udev/udevadm-settle.c
@@ -35,6 +35,7 @@
#include sys/types.h
#include udev.h
+#include util.h
static int adm_settle(struct udev *udev, int argc, char *argv[])
{
@@ -56,14 +57,19 @@ static int adm_settle(struct udev *udev, int argc, char
*argv[])
struct pollfd pfd[1] = { {.fd = -1}, };
struct udev_queue *udev_queue = NULL;
int rc = EXIT_FAILURE;
-
+int r = -1;
for (;;) {
int option;
int seconds;
option = getopt_long(argc, argv, s:e:t:E:qh, options, NULL);
-if (option == -1)
+if (option == -1) {
+if (optind argc) {
+fprintf(stderr, Extraneous argument:
'%s'\n,argv[optind]);
+exit(EXIT_FAILURE);
+}
break;
+}
switch (option) {
case 's':
@@ -73,11 +79,13 @@ static int adm_settle(struct udev *udev, int argc, char
*argv[])
end = strtoull(optarg, NULL, 0);
break;
case 't':
-seconds = atoi(optarg);
-if (seconds = 0)
+r = safe_atoi(optarg,seconds);
+if (r 0 || seconds 0) {
+fprintf(stderr, Invalid timeout value '%s':
%s\n, optarg, strerror(-r));
+exit(EXIT_FAILURE);
+} else {
timeout = seconds;
-else
-fprintf(stderr, invalid timeout value\n);
+}
break;
case 'q':
quiet = 1;
--
1.8.1
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH 2/2] add parameters checking for 'udevadm trigger'
This patch adds parameters checking for 'udevadm trigger'.
---
src/udev/udevadm-trigger.c | 22 +++---
1 file changed, 19 insertions(+), 3 deletions(-)
diff --git a/src/udev/udevadm-trigger.c b/src/udev/udevadm-trigger.c
index f472996..ee6a4a0 100644
--- a/src/udev/udevadm-trigger.c
+++ b/src/udev/udevadm-trigger.c
@@ -32,6 +32,7 @@
#include sys/un.h
#include udev.h
+#include util.h
static int verbose;
static int dry_run;
@@ -95,9 +96,12 @@ static int adm_trigger(struct udev *udev, int argc, char
*argv[])
TYPE_SUBSYSTEMS,
} device_type = TYPE_DEVICES;
const char *action = change;
+static const char action_table[] =
+add\0
+remove\0
+change\0;
struct udev_enumerate *udev_enumerate;
int rc = 0;
-
udev_enumerate = udev_enumerate_new(udev);
if (udev_enumerate == NULL) {
rc = 1;
@@ -111,8 +115,14 @@ static int adm_trigger(struct udev *udev, int argc, char
*argv[])
char buf[UTIL_PATH_SIZE];
option = getopt_long(argc, argv, vng:o:t:hc:p:s:S:a:A:y:b:,
options, NULL);
-if (option == -1)
+if (option == -1) {
+if (optind argc) {
+fprintf(stderr, Extraneous argument:
'%s'\n,argv[optind]);
+rc = 1;
+goto exit;
+}
break;
+}
switch (option) {
case 'v':
@@ -133,7 +143,13 @@ static int adm_trigger(struct udev *udev, int argc, char
*argv[])
}
break;
case 'c':
-action = optarg;
+if (!nulstr_contains(action_table,optarg)) {
+log_error(unknown action '%s'\n, optarg);
+rc = 2;
+goto exit;
+} else {
+action = optarg;
+}
break;
case 's':
udev_enumerate_add_match_subsystem(udev_enumerate,
optarg);
--
1.8.1
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH] man: fix typo
--- man/systemd-run.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/man/systemd-run.xml b/man/systemd-run.xml index 0881561..cc8a68a 100644 --- a/man/systemd-run.xml +++ b/man/systemd-run.xml @@ -60,7 +60,7 @@ along with systemd; If not, see http://www.gnu.org/licenses/. refsect1 titleDescription/title -paracommandsystemd-run/command may be used create and start +paracommandsystemd-run/command may be used to create and start a transient filename.service/filename or a filename.scope/filename unit and run the specified replaceableCOMMAND/replaceable in it./para -- 1.8.4.2 ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] Need advice on daemon's architecture
-Original Message- From: systemd-devel-boun...@lists.freedesktop.org [mailto:systemd-devel- boun...@lists.freedesktop.org] On Behalf Of Colin Guthrie Sent: Sunday, November 03, 2013 12:54 PM To: Peter Lemenkov; systemd-devel@lists.freedesktop.org Subject: Re: [systemd-devel] Need advice on daemon's architecture 'Twas brillig, and Peter Lemenkov at 03/11/13 06:40 did gyre and gimble: Hello All! I'm working on a system service which uses systemd intensively. Right now it's socket-activated, with main service of type simple. I recently added support for querying and publishing some internals via D-Bus, so it has a D-Bus name now. Does it add anything if I change type of a main service to dbus thus allowing systemd to know for sure if my service is fully initialized? If you are using systemd intensively, then you may want to use Type=notify. With type=dbus, systemd will consider things ready when you take the name on the bus, but this might not actually be the last thing you do to initialise your daemon (although if this is the only interface for clients this is not a problem!). You still might want to use sd_notify() instead. This can also pass through some internal performance info to systemd which will show up on systemctl status output which is kinda nice. Col -- Colin Guthrie gmane(at)colin.guthr.ie http://colin.guthr.ie/ Day Job: Tribalogic Limited http://www.tribalogic.net/ Open Source: Mageia Contributor http://www.mageia.org/ PulseAudio Hacker http://www.pulseaudio.org/ Trac Hacker http://trac.edgewall.org/ ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel Isn't the classical Linux way an option to? - the daemon does its initialization with the calling thread - once it is done with the initialization, it forks off a process that goes on with the daemons work (the main loop probably) - the calling thread returns, which signals systemd that the daemon is up now Type=forking must be defined in the .service to support this architecture. Are there any drawbacks with this solution? I'm just asking because I'm working at the moment on a daemon that is going exactly this way ... Best regards Marko Hoyer Software Group II (ADITG/SW2) Tel. +49 5121 49 6948 ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] tree-wide conversion from libdbus to libsystemd-bus
On Sunday, November 3, 2013, Daniel Mack wrote: On 10/30/2013 03:48 AM, Kay Sievers wrote: On Wed, Oct 23, 2013 at 1:22 AM, Kay Sievers k...@vrfy.orgjavascript:; wrote: [update] To avoid any duplication of work, here are the tools which still need conversion. Please reply to this mail, in case you decide to work on anything in that area. - timedatectl - systemd-logind - loginctl Peeters Simon: I'll take ... (probably loginctl afterwards) - localectl Kay will do that next - hostnamectl - pam_systemd Zbigniew: I'll do pam_systemd - systemctl I'll have a look at systemctl. Seems like a good way to get familiar with the new API. Might take me some days to finish it though. Daniel Yep, I've goy it nearly half finished by now, should be done by next week end. Marc-Antoine ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] Need advice on daemon's architecture
On Sunday 2013-11-03 14:42, Hoyer, Marko (ADITG/SW2) wrote: Isn't the classical Linux way an option to? - the daemon does its initialization with the calling thread - once it is done with the initialization, it forks off a process that goes on with the daemons work (the main loop probably) - the calling thread returns, which signals systemd that the daemon is up now Are there any drawbacks with this solution? Yes, you forgot the other 12 ugly steps as described in daemon(7). Which is why the classic way is not something worth the time. ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] Need advice on daemon's architecture
El 03/11/13 10:42, Hoyer, Marko (ADITG/SW2) escribió: Isn't the classical Linux way an option to? - the daemon does its initialization with the calling thread - once it is done with the initialization, it forks off a process that goes on with the daemons work (the main loop probably) - the calling thread returns, which signals systemd that the daemon is up now Type=forking must be defined in the .service to support this architecture. Are there any drawbacks with this solution? Yes, having reviewed dozens of daemons for migration to systemd, I can assure yours will also be missing something of the required initialization sequence (see daemon(7) ) or doing it wrong, as the number of daemons that do this routines correctly is almost non-existent. For new daemons, please use Type=notify. -- Judging by their response, the meanest thing you can do to people on the Internet is to give them really good software for free. - Anil Dash ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] Power aware units
Heya, systemd already allows launching specific tasks based on a timer, and intervals, and I was wondering whether power awareness was something planned for launching and stopping units. MacOS X 10.9 has some additional metadata for units that allows launchd to stop and start particular tasks based on power levels: http://arstechnica.com/apple/2013/10/os-x-10-9/16/ We could implement this in 2 ways: - systemd itself speaks over D-Bus to UPower (using the new DisplayDevice to merge UPS and batteries) and stop/starts the units. - systemd ships a set of units that UPower will launch/stop based on battery status. This would require UPower to know more about some other subsystems as well such as the lock screen status, or the hard drive state. This would be useful for things like backups, housekeeping (emptying old files from the trash, old thumbnails, etc.), launching update-db, etc. in addition to the simple timer intervals. I think the first option is the best one, as in addition to UPower, we'd need to talk to the kernel/udev (HDD spinning state), and logind (lock screen status). In addition to that, would it make sense for distributions to start porting their cron jobs to use systemd? Cheers ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] Power aware units
On Sun, Nov 3, 2013 at 3:36 PM, Bastien Nocera had...@hadess.net wrote: systemd already allows launching specific tasks based on a timer, and intervals, and I was wondering whether power awareness was something planned for launching and stopping units. MacOS X 10.9 has some additional metadata for units that allows launchd to stop and start particular tasks based on power levels: http://arstechnica.com/apple/2013/10/os-x-10-9/16/ We could implement this in 2 ways: - systemd itself speaks over D-Bus to UPower (using the new DisplayDevice to merge UPS and batteries) and stop/starts the units. - systemd ships a set of units that UPower will launch/stop based on battery status. This would require UPower to know more about some other subsystems as well such as the lock screen status, or the hard drive state. This would be useful for things like backups, housekeeping (emptying old files from the trash, old thumbnails, etc.), launching update-db, etc. in addition to the simple timer intervals. I think the first option is the best one, as in addition to UPower, we'd need to talk to the kernel/udev (HDD spinning state), and logind (lock screen status). Systemd should not get any direct or indirect dependency on upower for primary service management tasks. It just doesn't sound right to do dependencies in this direction. If systemd needs more information than the current on-battery vs. AC, we would need to add that to systemd itself, or even add support for a virtual/composite battery to the kernel. In addition to that, would it make sense for distributions to start porting their cron jobs to use systemd? This all sounds nice to have, and what we want to have in the longer run. We could do most of that already today, with just the battery/AC information, I think. Kay ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] Power aware units
On Sun, 2013-11-03 at 16:06 +0100, Kay Sievers wrote: On Sun, Nov 3, 2013 at 3:36 PM, Bastien Nocera had...@hadess.net wrote: systemd already allows launching specific tasks based on a timer, and intervals, and I was wondering whether power awareness was something planned for launching and stopping units. MacOS X 10.9 has some additional metadata for units that allows launchd to stop and start particular tasks based on power levels: http://arstechnica.com/apple/2013/10/os-x-10-9/16/ We could implement this in 2 ways: - systemd itself speaks over D-Bus to UPower (using the new DisplayDevice to merge UPS and batteries) and stop/starts the units. - systemd ships a set of units that UPower will launch/stop based on battery status. This would require UPower to know more about some other subsystems as well such as the lock screen status, or the hard drive state. This would be useful for things like backups, housekeeping (emptying old files from the trash, old thumbnails, etc.), launching update-db, etc. in addition to the simple timer intervals. I think the first option is the best one, as in addition to UPower, we'd need to talk to the kernel/udev (HDD spinning state), and logind (lock screen status). Systemd should not get any direct or indirect dependency on upower for primary service management tasks. It just doesn't sound right to do dependencies in this direction. That doesn't strike me as as crazy as it may seem. systemd would only monitor a couple of properties on a remote D-Bus name. If the name isn't present, use defaults (not on battery, not discharging, HDD spinning). If systemd needs more information than the current on-battery vs. AC, we would need to add that to systemd itself, or even add support for a virtual/composite battery to the kernel. That would leave policy handling (eg. what's low-battery?), and UPS support on the wayside. I also don't think you want to start polling batteries in systemd. In addition to that, would it make sense for distributions to start porting their cron jobs to use systemd? This all sounds nice to have, and what we want to have in the longer run. We could do most of that already today, with just the battery/AC information, I think. I think it's all or nothing. Merging batteries is a crappy enough job (with work-arounds for bad firmwares) that you really don't want it in the kernel or systemd itself. ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] Power aware units
On Sun, 2013-11-03 at 17:07 +0100, Bastien Nocera wrote: snip Systemd should not get any direct or indirect dependency on upower for primary service management tasks. It just doesn't sound right to do dependencies in this direction. That doesn't strike me as as crazy as it may seem. systemd would only monitor a couple of properties on a remote D-Bus name. If the name isn't present, use defaults (not on battery, not discharging, HDD spinning). FWIW, systemd would also need to pull in a dependency on logind. Maybe you're actually asking for UPower to be merged in systemd? That's probably do-able :) ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] tree-wide conversion from libdbus to libsystemd-bus
On 11/03/2013 03:12 PM, Marc-Antoine Perennou wrote: On Sunday, November 3, 2013, Daniel Mack wrote: On 10/30/2013 03:48 AM, Kay Sievers wrote: On Wed, Oct 23, 2013 at 1:22 AM, Kay Sievers k...@vrfy.org javascript:; wrote: [update] To avoid any duplication of work, here are the tools which still need conversion. Please reply to this mail, in case you decide to work on anything in that area. - timedatectl - systemd-logind - loginctl Peeters Simon: I'll take ... (probably loginctl afterwards) - localectl Kay will do that next - hostnamectl - pam_systemd Zbigniew: I'll do pam_systemd - systemctl I'll have a look at systemctl. Seems like a good way to get familiar with the new API. Might take me some days to finish it though. Daniel Yep, I've goy it nearly half finished by now, should be done by next week end. Ah, sorry. Missed your post on that. Alright, I'll trash my bits then and let you finished your work :) Daniel ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH 1/2] add parameters checking for 'udevadm settle
On Sun, Nov 03, 2013 at 07:49:02PM +0800, Yang Zhiyong wrote:
This patch adds parameters checking for 'udevadm settle'.
---
src/udev/udevadm-settle.c | 20 ++--
1 file changed, 14 insertions(+), 6 deletions(-)
diff --git a/src/udev/udevadm-settle.c b/src/udev/udevadm-settle.c
index c4fc4ee..06f4004 100644
--- a/src/udev/udevadm-settle.c
+++ b/src/udev/udevadm-settle.c
@@ -35,6 +35,7 @@
#include sys/types.h
#include udev.h
+#include util.h
static int adm_settle(struct udev *udev, int argc, char *argv[])
{
@@ -56,14 +57,19 @@ static int adm_settle(struct udev *udev, int argc, char
*argv[])
struct pollfd pfd[1] = { {.fd = -1}, };
struct udev_queue *udev_queue = NULL;
int rc = EXIT_FAILURE;
-
+int r = -1;
for (;;) {
int option;
int seconds;
option = getopt_long(argc, argv, s:e:t:E:qh, options,
NULL);
-if (option == -1)
+if (option == -1) {
+if (optind argc) {
+fprintf(stderr, Extraneous argument:
'%s'\n,argv[optind]);
+exit(EXIT_FAILURE);
+}
break;
+}
switch (option) {
case 's':
@@ -73,11 +79,13 @@ static int adm_settle(struct udev *udev, int argc, char
*argv[])
end = strtoull(optarg, NULL, 0);
break;
case 't':
-seconds = atoi(optarg);
-if (seconds = 0)
+r = safe_atoi(optarg,seconds);
So, why not make seconds unsigned? Actually, seconds is only used to set
timout, which is already unsigned. And safe_atou will not touch the destination
variable unless sucessfull, so you can simply do away with seconds,
making the code simpler.
+if (r 0 || seconds 0) {
+fprintf(stderr, Invalid timeout value '%s':
%s\n, optarg, strerror(-r));
+exit(EXIT_FAILURE);
+} else {
timeout = seconds;
-else
-fprintf(stderr, invalid timeout value\n);
+}
break;
case 'q':
quiet = 1;
Zbyszek
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] Need advice on daemon's architecture
-Original Message- From: systemd-devel-boun...@lists.freedesktop.org [mailto:systemd-devel- boun...@lists.freedesktop.org] On Behalf Of Cristian Rodríguez Sent: Sunday, November 03, 2013 3:25 PM To: systemd-devel@lists.freedesktop.org Subject: Re: [systemd-devel] Need advice on daemon's architecture El 03/11/13 10:42, Hoyer, Marko (ADITG/SW2) escribió: Isn't the classical Linux way an option to? - the daemon does its initialization with the calling thread - once it is done with the initialization, it forks off a process that goes on with the daemons work (the main loop probably) - the calling thread returns, which signals systemd that the daemon is up now Type=forking must be defined in the .service to support this architecture. Are there any drawbacks with this solution? Yes, having reviewed dozens of daemons for migration to systemd, I can assure yours will also be missing something of the required initialization sequence (see daemon(7) ) or doing it wrong, as the number of daemons that do this routines correctly is almost non-existent. For new daemons, please use Type=notify. -- Judging by their response, the meanest thing you can do to people on the Internet is to give them really good software for free. - Anil Dash ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel Thx for the fast feedback. Good hint with the man page, I'll have a more detailed look on the page. I think when you need to stay a bit independent from systemd and don't have a dbus interface which can be used for synchronization there is probably no other way then the classical one. But in case I'm starting in such a well prepared environment like the one provided by a systemd service, I hopefully will not run into any troubles even if my daemon is missing something of the required initialization sequence or doing it wrong ;) Best regards Marko Hoyer Software Group II (ADITG/SW2) Tel. +49 5121 49 6948 ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] Need advice on daemon's architecture
On Sun, Nov 03, 2013 at 06:18:39PM +, Hoyer, Marko (ADITG/SW2) wrote: Thx for the fast feedback. Good hint with the man page, I'll have a more detailed look on the page. I think when you need to stay a bit independent from systemd and don't have a dbus interface which can be used for synchronization there is probably no other way then the classical one. But in case I'm starting in such a well prepared environment like the one provided by a systemd service, I hopefully will not run into any troubles even if my daemon is missing something of the required initialization sequence or doing it wrong ;) Actually, the opposite is true. Systemd is more demanding, primarily because everything is faster, and if there's a race condition during daemon startup, it's more likely to matter with systemd. So, having a well prepared environment doesn't mean the startup procedure can be sloppy. Zbyszek ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH 1/2] add parameters checking for 'udevadm settle
-Original Message-
From: Zbigniew Jędrzejewski-Szmek [mailto:zbys...@in.waw.pl]
Sent: Monday, November 04, 2013 12:56 AM
To: Yang Zhiyong
Cc: systemd-devel@lists.freedesktop.org; k...@vrfy.org
Subject: Re: [systemd-devel][PATCH 1/2] add parameters checking for
'udevadm settle
On Sun, Nov 03, 2013 at 07:49:02PM +0800, Yang Zhiyong wrote:
This patch adds parameters checking for 'udevadm settle'.
---
src/udev/udevadm-settle.c | 20 ++--
1 file changed, 14 insertions(+), 6 deletions(-)
diff --git a/src/udev/udevadm-settle.c b/src/udev/udevadm-settle.c
index c4fc4ee..06f4004 100644
--- a/src/udev/udevadm-settle.c
+++ b/src/udev/udevadm-settle.c
@@ -35,6 +35,7 @@
#include sys/types.h
#include udev.h
+#include util.h
static int adm_settle(struct udev *udev, int argc, char *argv[]) {
@@ -56,14 +57,19 @@ static int adm_settle(struct udev *udev, int argc,
char *argv[])
struct pollfd pfd[1] = { {.fd = -1}, };
struct udev_queue *udev_queue = NULL;
int rc = EXIT_FAILURE;
-
+int r = -1;
for (;;) {
int option;
int seconds;
option = getopt_long(argc, argv, s:e:t:E:qh, options,
NULL);
-if (option == -1)
+if (option == -1) {
+if (optind argc) {
+fprintf(stderr, Extraneous argument:
'%s'\n,argv[optind]);
+exit(EXIT_FAILURE);
+}
break;
+}
switch (option) {
case 's':
@@ -73,11 +79,13 @@ static int adm_settle(struct udev *udev, int argc,
char *argv[])
end = strtoull(optarg, NULL, 0);
break;
case 't':
-seconds = atoi(optarg);
-if (seconds = 0)
+r = safe_atoi(optarg,seconds);
So, why not make seconds unsigned? Actually, seconds is only used to set
timout, which is already unsigned. And safe_atou will not touch the
destination variable unless sucessfull, so you can simply do away with
seconds, making the code simpler.
OK, I understand .safe_atou is more efficient.
Yang Zhiyong
+if (r 0 || seconds 0) {
+fprintf(stderr, Invalid timeout value
'%s': %s\n, optarg,
strerror(-r));
+exit(EXIT_FAILURE);
+} else {
timeout = seconds;
-else
-fprintf(stderr, invalid timeout
value\n);
+}
break;
case 'q':
quiet = 1;
Zbyszek
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH v3] add parameters checking for 'udevadm settle
This patch adds parameters checking for 'udevadm settle':
- Add extraneous argument checking.
If exist extraneous argument,the program will print stderr and exit.
- Change seconds's type from int to unsigned int.
And use safe_atou() for catching invalid timeout value.
---
src/udev/udevadm-settle.c | 22 +++---
1 file changed, 15 insertions(+), 7 deletions(-)
diff --git a/src/udev/udevadm-settle.c b/src/udev/udevadm-settle.c
index c4fc4ee..b991da2 100644
--- a/src/udev/udevadm-settle.c
+++ b/src/udev/udevadm-settle.c
@@ -35,6 +35,7 @@
#include sys/types.h
#include udev.h
+#include util.h
static int adm_settle(struct udev *udev, int argc, char *argv[])
{
@@ -56,14 +57,19 @@ static int adm_settle(struct udev *udev, int argc, char
*argv[])
struct pollfd pfd[1] = { {.fd = -1}, };
struct udev_queue *udev_queue = NULL;
int rc = EXIT_FAILURE;
-
+int r = -1;
for (;;) {
int option;
-int seconds;
+unsigned int seconds;
option = getopt_long(argc, argv, s:e:t:E:qh, options, NULL);
-if (option == -1)
+if (option == -1) {
+if (optind argc) {
+fprintf(stderr, Extraneous argument:
'%s'\n,argv[optind]);
+exit(EXIT_FAILURE);
+}
break;
+}
switch (option) {
case 's':
@@ -73,11 +79,13 @@ static int adm_settle(struct udev *udev, int argc, char
*argv[])
end = strtoull(optarg, NULL, 0);
break;
case 't':
-seconds = atoi(optarg);
-if (seconds = 0)
+r = safe_atou(optarg,seconds);
+if (r 0) {
+fprintf(stderr, Invalid timeout value '%s':
%s\n, optarg, strerror(-r));
+exit(EXIT_FAILURE);
+} else {
timeout = seconds;
-else
-fprintf(stderr, invalid timeout value\n);
+}
break;
case 'q':
quiet = 1;
--
1.8.1
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
