Re: [systemd-devel] [PATCH] udev: Remove hard-coded string offsets when parsing kernel command line.

2014-11-05 Thread David Herrmann 
Hi

On Wed, Nov 5, 2014 at 10:16 AM, Richard W.M. Jones rjo...@redhat.com wrote:
 ---
  src/udev/udevd.c | 23 ---
  1 file changed, 12 insertions(+), 11 deletions(-)

 diff --git a/src/udev/udevd.c b/src/udev/udevd.c
 index 193702c..340812b 100644
 --- a/src/udev/udevd.c
 +++ b/src/udev/udevd.c
 @@ -968,6 +968,7 @@ static void kernel_cmdline_options(struct udev *udev) {

  FOREACH_WORD_QUOTED(word, l, line, state) {
  char *s, *opt;
 +const char *name;

  s = strndup(word, l);
  if (!s)
 @@ -979,24 +980,24 @@ static void kernel_cmdline_options(struct udev *udev) {
  else
  opt = s;

 -if (startswith(opt, udev.log-priority=)) {
 +if (startswith(opt, name = udev.log-priority=)) {

This looks weird. Note that startswith() returns a pointer to the
offset, so maybe we should restructure like this:

if ((value = startswith(opt, udev.log-priority))) {
prio = util_log_priority(value);

}


I will try to fix this up myself. Lets see..

Thanks
David

  int prio;

 -prio = util_log_priority(opt + 18);
 +prio = util_log_priority(opt + strlen(name));
  log_set_max_level(prio);
  udev_set_log_priority(udev, prio);
 -} else if (startswith(opt, udev.children-max=)) {
 -r = safe_atoi(opt + 18, arg_children_max);
 +} else if (startswith(opt, name = udev.children-max=)) {
 +r = safe_atoi(opt + strlen(name), arg_children_max);
  if (r  0)
 -log_warning(Invalid udev.children-max 
 ignored: %s, opt + 18);
 -} else if (startswith(opt, udev.exec-delay=)) {
 -r = safe_atoi(opt + 16, arg_exec_delay);
 +log_warning(Invalid udev.children-max 
 ignored: %s, opt + strlen(name));
 +} else if (startswith(opt, name = udev.exec-delay=)) {
 +r = safe_atoi(opt + strlen(name), arg_exec_delay);
  if (r  0)
 -log_warning(Invalid udev.exec-delay 
 ignored: %s, opt + 16);
 -} else if (startswith(opt, udev.event-timeout=)) {
 -r = safe_atou64(opt + 19, arg_event_timeout_usec);
 +log_warning(Invalid udev.exec-delay 
 ignored: %s, opt + strlen(name));
 +} else if (startswith(opt, name = udev.event-timeout=)) {
 +r = safe_atou64(opt + strlen(name), 
 arg_event_timeout_usec);
  if (r  0) {
 -log_warning(Invalid udev.event-timeout 
 ignored: %s, opt + 19);
 +log_warning(Invalid udev.event-timeout 
 ignored: %s, opt + strlen(name));
  break;
  }
  arg_event_timeout_usec *= USEC_PER_SEC;
 --
 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] udev: Remove hard-coded string offsets when parsing kernel command line.

2014-11-05 Thread Richard W.M. Jones 
On Wed, Nov 05, 2014 at 12:54:21PM +0100, David Herrmann wrote:
 Hi
 
 On Wed, Nov 5, 2014 at 10:16 AM, Richard W.M. Jones rjo...@redhat.com wrote:
  ---
   src/udev/udevd.c | 23 ---
   1 file changed, 12 insertions(+), 11 deletions(-)
 
  diff --git a/src/udev/udevd.c b/src/udev/udevd.c
  index 193702c..340812b 100644
  --- a/src/udev/udevd.c
  +++ b/src/udev/udevd.c
  @@ -968,6 +968,7 @@ static void kernel_cmdline_options(struct udev *udev) {
 
   FOREACH_WORD_QUOTED(word, l, line, state) {
   char *s, *opt;
  +const char *name;
 
   s = strndup(word, l);
   if (!s)
  @@ -979,24 +980,24 @@ static void kernel_cmdline_options(struct udev *udev) 
  {
   else
   opt = s;
 
  -if (startswith(opt, udev.log-priority=)) {
  +if (startswith(opt, name = udev.log-priority=)) {
 
 This looks weird. Note that startswith() returns a pointer to the
 offset, so maybe we should restructure like this:
 
 if ((value = startswith(opt, udev.log-priority))) {
 prio = util_log_priority(value);
 
 }

Ah ... yes, that would be better.

Rich.

 I will try to fix this up myself. Lets see..
 
 Thanks
 David
 
   int prio;
 
  -prio = util_log_priority(opt + 18);
  +prio = util_log_priority(opt + strlen(name));
   log_set_max_level(prio);
   udev_set_log_priority(udev, prio);
  -} else if (startswith(opt, udev.children-max=)) {
  -r = safe_atoi(opt + 18, arg_children_max);
  +} else if (startswith(opt, name = udev.children-max=)) {
  +r = safe_atoi(opt + strlen(name), 
  arg_children_max);
   if (r  0)
  -log_warning(Invalid udev.children-max 
  ignored: %s, opt + 18);
  -} else if (startswith(opt, udev.exec-delay=)) {
  -r = safe_atoi(opt + 16, arg_exec_delay);
  +log_warning(Invalid udev.children-max 
  ignored: %s, opt + strlen(name));
  +} else if (startswith(opt, name = udev.exec-delay=)) {
  +r = safe_atoi(opt + strlen(name), arg_exec_delay);
   if (r  0)
  -log_warning(Invalid udev.exec-delay 
  ignored: %s, opt + 16);
  -} else if (startswith(opt, udev.event-timeout=)) {
  -r = safe_atou64(opt + 19, arg_event_timeout_usec);
  +log_warning(Invalid udev.exec-delay 
  ignored: %s, opt + strlen(name));
  +} else if (startswith(opt, name = udev.event-timeout=)) {
  +r = safe_atou64(opt + strlen(name), 
  arg_event_timeout_usec);
   if (r  0) {
  -log_warning(Invalid udev.event-timeout 
  ignored: %s, opt + 19);
  +log_warning(Invalid udev.event-timeout 
  ignored: %s, opt + strlen(name));
   break;
   }
   arg_event_timeout_usec *= USEC_PER_SEC;
  --
  2.1.0
 
  ___
  systemd-devel mailing list
  systemd-devel@lists.freedesktop.org
  http://lists.freedesktop.org/mailman/listinfo/systemd-devel

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-df lists disk usage of guests without needing to install any
software inside the virtual machine.  Supports Linux and Windows.
http://people.redhat.com/~rjones/virt-df/
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel