On Tue, 2 Apr 2024, Nam Cao wrote:
> Variable "optind" is used as the last argument of getopt_long(). This
> means it "is set to the index of the long option relative to longopts"
> (from man page). NULL check is then performed on argv[optind], which is
> not valid, because in this case, optind is not an index to argv[].
>
> There is another "optind" which is a global variable, which actually
> holds the index to argv[]. This is likely the actual intention here. By
> locally define another "optind", the "real optind" is shadowed in this
> scope.
>
> Furthermore, the original optind "is the index of the next element to
> be processed in argv" (from man page), not the index to the current
> element. So doing NULL-check on argv[optind] with the "original optind"
> is also not valid.
>
> There is no reason to do this NULL-check, since argv[optind] is not even
> read. Only optarg is read, which is never a NULL pointer in this case.
>
> Delete this incorrect and unnecessary "optind".
>
> Signed-off-by: Nam Cao <namcao@xxxxxxxxxxxxx>
> ---
> src/hackbench/hackbench.c | 10 ++++------
> 1 file changed, 4 insertions(+), 6 deletions(-)
>
> diff --git a/src/hackbench/hackbench.c b/src/hackbench/hackbench.c
> index 4430db0..d4924b3 100644
> --- a/src/hackbench/hackbench.c
> +++ b/src/hackbench/hackbench.c
> @@ -405,8 +405,6 @@ static unsigned int group(childinfo_t *child,
> static void process_options(int argc, char *argv[])
> {
> for(;;) {
> - int optind = 0;
> -
> static struct option longopts[] = {
> {"fds", required_argument, NULL, 'f'},
> {"fifo", no_argument, NULL, 'F'},
> @@ -422,13 +420,13 @@ static void process_options(int argc, char *argv[])
> };
>
> int c = getopt_long(argc, argv, "f:Fg:hl:pis:TP",
> - longopts, &optind);
> + longopts, NULL);
> if (c == -1) {
> break;
> }
> switch (c) {
> case 'f':
> - if (!(argv[optind] && (num_fds = atoi(optarg)) > 0)) {
> + if ((num_fds = atoi(optarg)) <= 0) {
> fprintf(stderr, "%s: --fds|-f requires an integer > 0\n", argv[0]);
> print_usage_exit(1);
> }
> @@ -437,7 +435,7 @@ static void process_options(int argc, char *argv[])
> fifo = 1;
> break;
> case 'g':
> - if (!(argv[optind] && (num_groups = atoi(optarg)) > 0)) {
> + if ((num_groups = atoi(optarg)) <= 0) {
> fprintf(stderr, "%s: --groups|-g requires an integer > 0\n", argv[0]);
> print_usage_exit(1);
> }
> @@ -446,7 +444,7 @@ static void process_options(int argc, char *argv[])
> print_usage_exit(0);
> break;
> case 'l':
> - if (!(argv[optind] && (loops = atoi(optarg)) > 0)) {
> + if ((loops = atoi(optarg)) <= 0) {
> fprintf(stderr, "%s: --loops|-l requires an integer > 0\n", argv[0]);
> print_usage_exit(1);
> }
> --
Signed-off-by: John Kacur <jkacur@xxxxxxxxxx>
