On 02/01/21 17:12, Karel Zak wrote:
> On Sat, Jan 30, 2021 at 08:50:36PM +0000, Qais Yousef wrote:
>
> Now I see that you have reused chrt concept of --pid. We keep it for
> chrt due to backward compatibility, but for new util it would be
> better to use standard 'required_argument' for --pid.
Hmm what does required mean here? --pid is optional. But if passed, a value
is required to be passed indeed.
>
> > + static const struct option longopts[] = {
> > + { "all-tasks", no_argument, NULL, 'a' },
> > + { "pid", no_argument, NULL, 'p' },
>
> { "pid", required_argument, NULL, 'p' },
Assuming this means --pid must be followed by a value then indeed that's what
we expect to happen.
>
> > + { "system", no_argument, NULL, 's' },
> > + { "reset-on-fork", no_argument, NULL, 'R' },
> > + { "help", no_argument, NULL, 'h' },
> > + { "verbose", no_argument, NULL, 'v' },
> > + { "version", no_argument, NULL, 'V' },
> > + { NULL, no_argument, NULL, 0 }
> > + };
> > +
> > + setlocale(LC_ALL, "");
> > + bindtextdomain(PACKAGE, LOCALEDIR);
> > + textdomain(PACKAGE);
> > + close_stdout_atexit();
> > +
> > + while((c = getopt_long(argc, argv, "+asRphmMvV", longopts, NULL)) != -1)
>
> The same we can do for -m and -M, just normal options where getopt_long()
> use optarg:
>
> "+asRp:hm:M:vV"
+1
>
> > + {
> > + switch (c) {
> > + case 'a':
> > + ctl->all_tasks = 1;
> > + break;
> > + case 'p':
> > + errno = 0;
> > + ctl->pid = strtos32_or_err(argv[optind], _("invalid PID argument"));
>
>
> ctl->pid = strtos32_or_err(optarg, _("invalid PID argument"));
+1
will fix this and the 2 other occurrences below to include in v3.
Thanks!
--
Qais Yousef
>
> > + optind++;
> > + break;
> > + case 's':
> > + ctl->system = 1;
> > + break;
> > + case 'R':
> > + ctl->reset_on_fork = 1;
> > + break;
> > + case 'v':
> > + ctl->verbose = 1;
> > + break;
> > + case 'm':
> > + ctl->util_min = strtos32_or_err(argv[optind], _("invalid util_min argument"));
>
> ctl->util_min = strtos32_or_err(optarg, _("invalid util_min argument"));
>
>
> > + ctl->util_min_set = 1;
> > + validate_util(ctl->util_min);
> > + optind++;
> > + break;
> > + case 'M':
> > + ctl->util_max = strtos32_or_err(argv[optind], _("invalid util_max argument"));
>
> ctl->util_max = strtos32_or_err(optarg, _("invalid util_max argument"));
>
> > + ctl->util_max_set = 1;
> > + validate_util(ctl->util_max);
> > + optind++;
> > + break;
> > + case 'V':
> > + print_version(EXIT_SUCCESS);
> > + /* fallthrough */
> > + case 'h':
> > + usage();
> > + default:
> > + errtryhelp(EXIT_FAILURE);
> > + }
> > + }
>
>
> Karel
>
>
> --
> Karel Zak <kzak@xxxxxxxxxx>
> http://karelzak.blogspot.com
>
