diff mbox series

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

Message ID 20240416202841.725706-3-aalbersh@redhat.com (mailing list archive)
State Superseded
Headers show
Series Refactoring from Coverity scan fixes | expand

Commit Message

Andrey Albershteyn April 16, 2024, 8:28 p.m. UTC
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(-)

Comments

Darrick J. Wong April 16, 2024, 8:49 p.m. UTC | #1
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
> 
>
diff mbox series

Patch

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);
+			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 */