Tobias Stoeckmann wrote: > On Sat, Dec 05, 2015 at 06:26:35AM -0500, Ted Unangst wrote: > > may i suggest strlen(s) instead of strchr(s, 0)? > > There's actually one part in newfs' code that uses this. And in theory > it has the same issue, not checking if s (which is special, which might > be argv[0]) is empty. I highly doubt this could be reached there, but > I fixed it anyway. Until now it uses strncpy, and with the switch to > strlcpy this is just another additional boundary check in place.
ok mmcc@ It'd be nice to have a macro for specname's size, IMO. Using sizeof() in strlcpy makes me uneasy and is less readable. That's a separate improvement, though. > Index: sbin/newfs/newfs.c > =================================================================== > RCS file: /cvs/src/sbin/newfs/newfs.c,v > retrieving revision 1.103 > diff -u -p -u -p -r1.103 newfs.c > --- sbin/newfs/newfs.c 25 Nov 2015 19:45:21 -0000 1.103 > +++ sbin/newfs/newfs.c 5 Dec 2015 12:32:07 -0000 > @@ -423,10 +423,11 @@ main(int argc, char *argv[]) > warnx("%s: not a character-special device", > special); > } > - cp = strchr(argv[0], '\0') - 1; > - if (cp == NULL || > - ((*cp < 'a' || *cp > ('a' + maxpartitions - 1)) > - && !isdigit((unsigned char)*cp))) > + if (*argv[0] == '\0') > + fatal("empty partition name supplied"); > + cp = argv[0] + strlen(argv[0]) - 1; > + if ((*cp < 'a' || *cp > ('a' + maxpartitions - 1)) > + && !isdigit((unsigned char)*cp)) > fatal("%s: can't figure out file system partition", > argv[0]); > lp = getdisklabel(special, fsi); > @@ -631,8 +632,9 @@ rewritelabel(char *s, int fd, struct dis > /* > * Make name for 'c' partition. > */ > - strncpy(specname, s, sizeof(specname) - 1); > - specname[sizeof(specname) - 1] = '\0'; > + if (*s == '\0' || > + strlcpy(specname, s, sizeof(specname)) >= sizeof(specname)) > + fatal("%s: invalid partition name supplied", s); > cp = specname + strlen(specname) - 1; > if (!isdigit((unsigned char)*cp)) > *cp = 'c'; > Index: sbin/newfs_ext2fs/newfs_ext2fs.c > =================================================================== > RCS file: /cvs/src/sbin/newfs_ext2fs/newfs_ext2fs.c,v > retrieving revision 1.21 > diff -u -p -u -p -r1.21 newfs_ext2fs.c > --- sbin/newfs_ext2fs/newfs_ext2fs.c 28 Nov 2015 06:12:09 -0000 1.21 > +++ sbin/newfs_ext2fs/newfs_ext2fs.c 5 Dec 2015 12:32:07 -0000 > @@ -529,9 +529,11 @@ getpartition(int fsi, const char *specia > errx(EXIT_FAILURE, "%s: block device", special); > if (!S_ISCHR(st.st_mode)) > warnx("%s: not a character-special device", special); > - cp = strchr(argv[0], '\0') - 1; > - if (cp == NULL || ((*cp < 'a' || *cp > ('a' + getmaxpartitions() - 1)) > - && !isdigit((unsigned char)*cp))) > + if (*argv[0] == '\0') > + errx(EXIT_FAILURE, "empty partition name supplied"); > + cp = argv[0] + strlen(argv[0]) - 1; > + if ((*cp < 'a' || *cp > ('a' + getmaxpartitions() - 1)) > + && !isdigit((unsigned char)*cp)) > errx(EXIT_FAILURE, "%s: can't figure out file system > partition", argv[0]); > lp = getdisklabel(special, fsi); > if (isdigit((unsigned char)*cp)) >