[PATCH v2 3/3] touch: add --time=what option

Xabier Oneca -- xOneca xoneca at gmail.com
Tue Apr 13 21:36:58 UTC 2021


Hi Denys,

> >  #if ENABLE_FEATURE_TOUCH_SUSV3
> >      char *reference_file = NULL;
> >      char *date_str = NULL;
> > +    IF_LONG_OPTS(char *time_arg = NULL;)
>
> These are actually long-ish insns
> (on x86, "mov immediate" (and "test") has no byte-extending version,
> unlike ALU ops). Sometimes it's cheaper to test
> if (opt & OPT_l)...   than  if (optstr)...
> - the former does not require NULLing.

While working with the implementation of -a/-m I saw the very
opposite, so here I opted for the same strategy, without even trying
the way you suggest here. Good to know for the future.

> > +        static const char *time_values ALIGN1 =
> > +            /* OPT_a: */ "access\0" "atime\0" "use\0"
> > +            /* OPT_m: */ "modify\0" "mtime\0";
> > +        int pos = index_in_substrings(time_values, time_arg);
> > +
> > +        if (pos >= 3) {
> > +            opts |= OPT_m;
> > +        } else if (pos >= 0) {
> > +            opts |= OPT_a;
> > +        } else {
> > +            //bb_error_msg("Invalid value: %s", time_arg);
> > +            bb_show_usage();
> > +        }
>
> how about this instead? -
>            opts |= (time_arg[0] == 'm') ? OPT_m : OPT_a;

Your suggestion is pretty smart. But I don't know... If it were a more
popular option maybe. But I don't like it 'half baked'.

This option being neither popular[citation needed] nor part of the
standard, if I had to vote I would reject it entirely. I'm guilty of
featuritis. Sorry. :)

Cheers,

Xabier Oneca_,,_


More information about the busybox mailing list