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

[YangZhiyong]

> -----邮件原件-----
> 发件人: Zbigniew Jędrzejewski-Szmek [mailto:zbyszek at in.waw.pl]
> 发送时间: 2013年10月27日 3:29
> 收件人: Yang Zhiyong
> 抄送: systemd-devel at lists.freedesktop.org; kay at vrfy.org
> 主题: 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.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.

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.


> 
> >                  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() ?
Shall I write like this:
                    case 't':
                        if (safe_atoi(optarg,&seconds) < 0){
                                fprintf(stderr, "invalid timeout value\n");
                                exit(EXIT_FAILURE);
                        }
                        break;

> 
> Zbyszek