Re: [PATCH 1/2] xfs_fsr: replace atoi() with strtol()

Darrick J. Wong
2024-04-16T20:49:11Z

On Tue, Apr 16, 2024 at 10:28:41PM +0200, Andrey Albershteyn wrote:
> Replace atoi() which silently fails with strtol() and report the
> error.
>
> Signed-off-by: Andrey Albershteyn <aalbersh@redhat.com>
> ---
> fsr/xfs_fsr.c | 23 ++++++++++++++++++++---
> 1 file changed, 20 insertions(+), 3 deletions(-)
>
> diff --git a/fsr/xfs_fsr.c b/fsr/xfs_fsr.c
> index 4e29a8a2c548..5fabc965183e 100644
> --- a/fsr/xfs_fsr.c
> +++ b/fsr/xfs_fsr.c
> @@ -164,7 +164,12 @@ main(int argc, char **argv)
> usage(1);
> break;
> case 't':
> - howlong = atoi(optarg);
> + howlong = strtol(optarg, NULL, 10);

C library functions don't clear errno; they only set it after something
goes wrong. For functions that don't return -1 to indicate that
something went wrong, you have to clear errno explicitly before calling
the function:

errno = 0;
howlong = strtol(optarg, NULL, 10);
if (errno)
fprintf(...);

If you don't clear it, you can then pick up an errno set by some past
library call, which can lead to strange error messages.

--D

> + if (errno) {
> + fprintf(stderr, _("%s: invalid interval: %s\n"),
> + progname, strerror(errno));
> + exit(1);
> + }
> if (howlong > INT_MAX) {
> fprintf(stderr, _("%s: too long\n"), progname);
> exit(1);
> @@ -177,10 +182,22 @@ main(int argc, char **argv)
> mtab = optarg;
> break;
> case 'b':
> - argv_blksz_dio = atoi(optarg);
> + argv_blksz_dio = strtol(optarg, NULL, 10);
> + if (errno) {
> + fprintf(stderr,
> + _("%s: invalid block size: %s\n"),
> + progname, strerror(errno));
> + exit(1);
> + }
> break;
> case 'p':
> - npasses = atoi(optarg);
> + npasses = strtol(optarg, NULL, 10);
> + if (errno) {
> + fprintf(stderr,
> + _("%s: invalid number of passes: %s\n"),
> + progname, strerror(errno));
> + exit(1);
> + }
> break;
> case 'C':
> /* Testing opt: coerses frag count in result */
> --
> 2.42.0
>
>