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

[Zbigniew Jędrzejewski-Szmek]

On Wed, Oct 23, 2013 at 03:09:37PM +0800, Yang Zhiyong wrote:
> This patch adds parameters checking for 'udevadm trigger'
Similar comments as for the other patch.

> Signed-off-by: Yang Zhiyong <yangzy.fnst at cn.fujitsu.com>
Not needed.

> ---
>  src/udev/udevadm-trigger.c |   16 ++++++++++++++--
>  1 files changed, 14 insertions(+), 2 deletions(-)
>  mode change 100644 => 100755 src/udev/udevadm-trigger.c
> 
> diff --git a/src/udev/udevadm-trigger.c b/src/udev/udevadm-trigger.c
> old mode 100644
> new mode 100755
> index f472996..2c0f86d
> --- a/src/udev/udevadm-trigger.c
> +++ b/src/udev/udevadm-trigger.c
> @@ -111,8 +111,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, "unknown option\n");
> +                                rc = 1;
> +                                goto exit;
> +                        }
>                          break;
> + 
-1 is no more args.

>                  switch (option) {
>                  case 'v':
> @@ -133,7 +139,13 @@ static int adm_trigger(struct udev *udev, int argc, char *argv[])
>                          }
>                          break;
>                  case 'c':
> -                        action = optarg;
> +                        if (streq(optarg, "add") || streq(optarg, "change") || streq(optarg, "remove")) {
> +                                action = optarg;
> +                        } else {
> +                                log_error("unknown action '%s'\n", optarg);
> +                                rc = 2;
> +                                goto exit;
> +                        }
Here please use the normal systemd style of 
  action
  check
    return
  next-action

i.e. something like (untested):
case 'c':
  if (!nulstr_contains("add\0change\0remove\0", optarg)) {
     log_error(...)
     ...
  }

  action = optarg;
  break;

Please resubmit.

Zbyszek