[systemd-devel] [PATCH 1/2]add parameters checking for 'udevadm settle'

[Zbigniew Jędrzejewski-Szmek]

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.fnst at 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.

>                  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().

Zbyszek