Re: Incorrect usage of strtol, atoi for non-numeric junk inputs

On Thu, Jul 08, 2021 at 05:30:23PM +0900, Kyotaro Horiguchi wrote:
> Looked through the three threads.

Thanks!

> [1] is trying to expose pg_strtoint16/32 to frontend, but I don't see
> much point in doing that in conjunction with [2] or this thread. Since
> the integral parameter values of pg-commands are in int, which the
> exising function strtoint() is sufficient to read. So even [2] itself
> doesn't need to utilize [1].

It sounds sensible from here to just use strtoint(), some strtol(),
son strtod() and call it a day as these are already available.

> - wait_seconds = atoi(optarg);
> + errno = 0;
> + wait_seconds = strtoint(optarg, &endptr, 10);
> + if (*endptr || errno == ERANGE || wait_seconds < 0)
> + {
> + pg_log_error("invalid timeout \"%s\"", optarg);
> + exit(1);
> + }
> [ ... ]
> - killproc = atol(argv[++optind]);
> + errno = 0;
> + killproc = strtol(argv[++optind], &endptr, 10);
> + if (*endptr || errno == ERANGE || killproc < 0)
> + {
> + pg_log_error("invalid process ID \"%s\"", argv[optind]);
> + exit(1);
> + }

Er, wait. We've actually allowed negative values for pg_ctl
--timeout or the subcommand kill!?

> case 'j':
> - user_opts.jobs = atoi(optarg);
> + errno = 0;
> + user_opts.jobs = strtoint(optarg, &endptr, 10);
> + /**/
> + if (*endptr || errno == ERANGE)
> + pg_fatal("invalid number of jobs %s\n", optarg);
> +
> break;

This one in pg_upgrade is incomplete. Perhaps the missing comment
should tell that negative job values are checked later on?
--
Michael