cvs commit: src/sbin/badsect badsect.c

[Giorgos Keramidas]

On 2005-01-03 13:16, Scott Long <scottl at freebsd.org> wrote:
>Robert Watson wrote:
>>rwatson     2005-01-03 19:03:40 UTC
>>
>>  FreeBSD src repository
>>
>>  Modified files:
>>    sbin/badsect         badsect.c
>>  Log:
>>  The badsect(8) utility uses atol(), which doesn't allow very good error
>>  checking and only recognizes numbers in base 10.  The attached patch
>>  checks errno after strtol() and uses a base of 0 to allow octal, or hex
>>  sector numbers too.
>>
>>  PR:             73112
>>  Submitted by:   keramida
>>  MFC after:      2 weeks
>>
>>  Revision  Changes    Path
>>  1.21      +4 -1      src/sbin/badsect/badsect.c
>
> I'd argue that atol() is buggy for a number of other reasons also,
> mainly that it means that you're limited to 2^31 sectors.  Maybe this
> should change to strtoull() ?

Ideally, if that's not a problem, we should go for uintmax_t, since
`number' in this case is then stored in a daddr_t, which is 64-bits
wide.

How about something like this?  I could find no easy way to check for
overflow of daddr_t, so this is still not quite perfect but it's an
improvement that may help with Really-Huge(TM) disks.

%%%
Index: badsect.c
===================================================================
RCS file: /home/ncvs/src/sbin/badsect/badsect.c,v
retrieving revision 1.21
diff -u -u -r1.21 badsect.c
--- badsect.c	3 Jan 2005 19:03:40 -0000	1.21
+++ badsect.c	5 Jan 2005 15:03:39 -0000
@@ -62,6 +62,7 @@
 #include <errno.h>
 #include <dirent.h>
 #include <fcntl.h>
+#include <inttypes.h>
 #include <libufs.h>
 #include <paths.h>
 #include <stdio.h>
@@ -94,6 +95,7 @@
 	DIR *dirp;
 	char name[2 * MAXPATHLEN];
 	char *name_dir_end;
+	char *errp;
 
 	if (argc < 3)
 		usage();
@@ -124,8 +126,11 @@
 			err(7, "%s", name);
 	}
 	for (argc -= 2, argv += 2; argc > 0; argc--, argv++) {
-		number = strtol(*argv, NULL, 0);
-		if (errno == EINVAL || errno == ERANGE)
+		errp = NULL;
+		errno = 0;
+		number = strtoumax(*argv, &errp, 0);
+		if (errno != 0 || errp != NULL || (errp != NULL &&
+		    *errp != '\0' && *argv != NULL && **argv != '\0'))
 			err(8, "%s", *argv);
 		if (chkuse(number, 1))
 			continue;
%%%