A few WARNS6 cleanups

Chris Pressey cpressey at catseye.mine.nu
Sun Jan 2 14:11:25 PST 2005


On Sun, 2 Jan 2005 12:00:52 +0100
Peter Schuller <peter.schuller at xxxxxxxxxxxx> wrote:

> > > iostat: minor changes required
> > 
> > Committed, with a minor change: the extra function doesn't seem to
> > be necessary, the parameter can just be marked as unused.  Thanks!
> 
> Ok. The reason I added it was that I felt it was extremely ugly to
> have phdr() be called with a 'dummy' argument in the rest of the code.

Yeah, it is ugly.  Can you suggest a better name for the function than
'phdr' ?  :)

> > This one also looks good, but since it's more involved, I'll play
> > with it a bit more before committing it.  I suspect a lot of the
> > signed/unsigned things can be resolved without adding casts, by
> > making the loop variables unsigned, since they usually range only
> > from 0 to some positive integer anyway.
> 
> I also realized I accidentally left a comment that was meant to be
> temporary (//CLEANUP....).

No problem.  Patch with my changes applied is attached.

> Anyways. Will be a bit more agressive in the future instead of just
> adding casts. The idea was to be conservative in that the reason for
> the original choice for the size and/or signedness of a variable may
> be important but not necessarily readily apparant. So addings casts
> felt less intrusive. In hindsight that was probably not a good idea.

Actually, only one of them turned out to be problematic - I was wrong
about the loop variables in this instance, as many were counting down
(to below 0) as were counting up...

You're right about the underlying cause - there are a lot of variables
where it doesn't make sense for them ever to be negative (block size,
for instance), but they're all declared as plain ints.  This might be
because that's how they're stored on disk - or not - either way it will
be non-trivial to fix this correctly.  The casts are good enough for
now, I think, so unless someone says otherwise, I'll commit this in a
day or so.

-Chris

Index: sbin/newfs/Makefile
===================================================================
RCS file: /home/dcvs/src/sbin/newfs/Makefile,v
retrieving revision 1.3
diff -u -r1.3 Makefile
--- sbin/newfs/Makefile	1 Dec 2003 04:35:39 -0000	1.3
+++ sbin/newfs/Makefile	2 Jan 2005 18:55:07 -0000
@@ -5,6 +5,7 @@
 PROG=	newfs
 SRCS=	getmntopts.c newfs.c mkfs.c fscopy.c
 MAN=	newfs.8
+WARNS?= 6
 
 MOUNT=	${.CURDIR}/../mount
 CFLAGS+=-DMFS -DFSIRAND -I${MOUNT}
Index: sbin/newfs/fscopy.c
===================================================================
RCS file: /home/dcvs/src/sbin/newfs/fscopy.c,v
retrieving revision 1.4
diff -u -r1.4 fscopy.c
--- sbin/newfs/fscopy.c	26 Jun 2004 22:54:01 -0000	1.4
+++ sbin/newfs/fscopy.c	2 Jan 2005 18:55:07 -0000
@@ -51,6 +51,8 @@
     char		fs_Name[4];
 };
 
+static char empty_string[] = "";
+
 static
 fsnode_t
 fsmknode(const char *path)
@@ -72,7 +74,7 @@
     return(node);
 }
 
-fsnode_t
+static fsnode_t
 fsgethlink(fsnode_t hlinks, fsnode_t node)
 {
     fsnode_t scan;
@@ -87,7 +89,7 @@
     return(NULL);
 }
 
-char *
+static char *
 fshardpath(fsnode_t hlink, fsnode_t node)
 {
     fsnode_t scan;
@@ -221,7 +223,7 @@
 		    node->fs_Data[n] = 0;
 		}
 	    } else if (n == 0) {
-		node->fs_Data = "";
+		node->fs_Data = empty_string;
 		node->fs_Bytes = 0;
 	    } else {
 		fprintf(stderr, "Unable to read link: %s\n", path);
Index: sbin/newfs/mkfs.c
===================================================================
RCS file: /home/dcvs/src/sbin/newfs/mkfs.c,v
retrieving revision 1.9
diff -u -r1.9 mkfs.c
--- sbin/newfs/mkfs.c	18 Dec 2004 21:43:39 -0000	1.9
+++ sbin/newfs/mkfs.c	2 Jan 2005 21:40:02 -0000
@@ -43,13 +43,14 @@
 
 extern int atoi(char *);
 extern char * getenv(char *);
-#endif
 
 #ifdef FSIRAND
 extern long random(void);
 extern void srandomdev(void);
 #endif
 
+#endif /* STANDALONE */
+
 /*
  * make file system for cylinder-group style file systems
  */
@@ -128,9 +129,9 @@
 #ifdef FSIRAND
 int     randinit;
 #endif
-daddr_t	alloc();
-long	calcipg();
-static int charsperline();
+daddr_t	alloc(int size, int mode);
+long	calcipg(long cylspg, long bpcg, off_t *usedbp);
+static int charsperline(void);
 void clrblock(struct fs *, unsigned char *, int);
 void fsinit(time_t);
 void initcg(int, time_t);
@@ -152,14 +153,17 @@
 caddr_t realloc(char *, u_long);
 #endif
 
+void started(int signo);
+void parentready(int signo);
+
 int mfs_ppid = 0;
 int parentready_signalled;
 
 void
 mkfs(struct partition *pp, char *fsys, int fi, int fo, const char *mfscopy)
 {
-	register long i, mincpc, mincpg, inospercg;
-	long cylno, rpos, blk, j, warn = 0;
+	long i, mincpc, mincpg, inospercg;
+	long cylno, rpos, blk, j, emitwarn = 0;
 	long used, mincpgcnt, bpcg;
 	off_t usedb;
 	long mapcramped, inodecramped;
@@ -167,8 +171,6 @@
 	int status, fd;
 	time_t utime;
 	quad_t sizepb;
-	void started();
-	void parentready();
 	int width;
 	char tmpbuf[100];	/* XXX this will break in about 2,500 years */
 
@@ -183,17 +185,18 @@
 #endif
 	if (mfs) {
 		int omask;
+		pid_t child;
 
 		mfs_ppid = getpid();
 		signal(SIGUSR1, parentready);
-		if ((i = fork())) {
-			if (i == -1)
+		if ((child = fork()) != 0) {
+			if (child == -1)
 				err(10, "mfs");
 			if (mfscopy)
 			    copyroot = FSCopy(&copyhlinks, mfscopy);
 			signal(SIGUSR1, started);
-			kill(i, SIGUSR1);
-			if (waitpid(i, &status, 0) != -1 && WIFEXITED(status))
+			kill(child, SIGUSR1);
+			if (waitpid(child, &status, 0) != -1 && WIFEXITED(status))
 				exit(WEXITSTATUS(status));
 			exit(11);
 			/* NOTREACHED */
@@ -207,17 +210,22 @@
 #else
 		raise_data_limit();
 #endif
-		if(filename) {
+		if (filename != NULL) {
 			unsigned char buf[BUFSIZ];
-			unsigned long l,l1;
-			fd = open(filename,O_RDWR|O_TRUNC|O_CREAT,0644);
+			unsigned long l, l1;
+			ssize_t w;
+
+			fd = open(filename, O_RDWR|O_TRUNC|O_CREAT, 0644);
 			if(fd < 0)
 				err(12, "%s", filename);
-			for(l=0;l< fssize * sectorsize;l += l1) {
+			for (l = 0;
+			     l < (u_long)fssize * (u_long)sectorsize;
+			     l += l1) {
 				l1 = fssize * sectorsize;
 				if (BUFSIZ < l1)
 					l1 = BUFSIZ;
-				if (l1 != write(fd,buf,l1))
+				w = write(fd, buf, l1);
+				if (w < 0 || l1 != (u_long)w)
 					err(12, "%s", filename);
 			}
 			membase = mmap(
@@ -234,7 +242,8 @@
 #ifndef STANDALONE
 			get_memleft();
 #endif
-			if (fssize * sectorsize > (memleft - 131072))
+			if ((u_long)fssize * (u_long)sectorsize >
+			    (memleft - 131072))
 				fssize = (memleft - 131072) / sectorsize;
 			if ((membase = malloc(fssize * sectorsize)) == NULL)
 				errx(13, "malloc failed");
@@ -375,7 +384,7 @@
 	if (maxcontig > 1)
 		sblock.fs_contigsumsize = MIN(maxcontig, FS_MAXCONTIG);
 	mapcramped = 0;
-	while (CGSIZE(&sblock) > sblock.fs_bsize) {
+	while (CGSIZE(&sblock) > (uint32_t)sblock.fs_bsize) {
 		mapcramped = 1;
 		if (sblock.fs_bsize < MAXBSIZE) {
 			sblock.fs_bsize <<= 1;
@@ -422,7 +431,7 @@
 		sblock.fs_fragshift -= 1;
 		mincpc >>= 1;
 		sblock.fs_cpg = roundup(mincpgcnt, mincpc);
-		if (CGSIZE(&sblock) > sblock.fs_bsize) {
+		if (CGSIZE(&sblock) > (uint32_t)sblock.fs_bsize) {
 			sblock.fs_bsize <<= 1;
 			break;
 		}
@@ -478,7 +487,7 @@
 	/*
 	 * Must ensure there is enough space to hold block map.
 	 */
-	while (CGSIZE(&sblock) > sblock.fs_bsize) {
+	while (CGSIZE(&sblock) > (uint32_t)sblock.fs_bsize) {
 		mapcramped = 1;
 		sblock.fs_cpg -= mincpc;
 		sblock.fs_ipg = calcipg(sblock.fs_cpg, bpcg, &usedb);
@@ -518,7 +527,7 @@
 	sblock.fs_ncyl = fssize * NSPF(&sblock) / sblock.fs_spc;
 	if (fssize * NSPF(&sblock) > sblock.fs_ncyl * sblock.fs_spc) {
 		sblock.fs_ncyl++;
-		warn = 1;
+		emitwarn = 1;
 	}
 	if (sblock.fs_ncyl < 1) {
 		printf("file systems must have at least one cylinder\n");
@@ -630,9 +639,9 @@
 		sblock.fs_ncyl -= sblock.fs_ncyl % sblock.fs_cpg;
 		sblock.fs_size = fssize = sblock.fs_ncyl * sblock.fs_spc /
 		    NSPF(&sblock);
-		warn = 0;
+		emitwarn = 0;
 	}
-	if (warn && !mfs) {
+	if (emitwarn && !mfs) {
 		printf("Warning: %d sector(s) in last cylinder unallocated\n",
 		    sblock.fs_spc -
 		    (fssize * NSPF(&sblock) - (sblock.fs_ncyl - 1)
@@ -768,9 +777,10 @@
 {
 	daddr_t cbase, d, dlower, dupper, dmax, blkno;
 	long i;
-	register struct csum *cs;
+	unsigned long k;
+	struct csum *cs;
 #ifdef FSIRAND
-	long j;
+	uint32_t j;
 #endif
 
 	/*
@@ -823,15 +833,19 @@
 		exit(37);
 	}
 	acg.cg_cs.cs_nifree += sblock.fs_ipg;
-	if (cylno == 0)
-		for (i = 0; i < ROOTINO; i++) {
-			setbit(cg_inosused(&acg), i);
+	if (cylno == 0) {
+		for (k = 0; k < ROOTINO; k++) {
+			setbit(cg_inosused(&acg), k);
 			acg.cg_cs.cs_nifree--;
 		}
+	}
 	for (i = 0; i < sblock.fs_ipg / INOPF(&sblock); i += sblock.fs_frag) {
 #ifdef FSIRAND
-		for (j = 0; j < sblock.fs_bsize / sizeof(struct dinode); j++)
+		for (j = 0;
+		     j < sblock.fs_bsize / sizeof(struct dinode);
+		     j++) {
 			zino[j].di_gen = random();
+		}
 #endif
 		wtfs(fsbtodb(&sblock, cgimin(&sblock, cylno) + i),
 		    sblock.fs_bsize, (char *)zino);
@@ -1020,7 +1034,7 @@
  * return size of directory.
  */
 int
-makedir(register struct direct *protodir, int entries)
+makedir(struct direct *protodir, int entries)
 {
 	char *cp;
 	int i, spcleft;
@@ -1094,7 +1108,7 @@
  * Calculate number of inodes per group.
  */
 long
-calcipg(long cpg, long bpcg, off_t *usedbp)
+calcipg(long cylspg, long bpcg, off_t *usedbp)
 {
 	int i;
 	long ipg, new_ipg, ncg, ncyl;
@@ -1105,7 +1119,7 @@
 	 * Note that fssize is still in sectors, not filesystem blocks.
 	 */
 	ncyl = howmany(fssize, (u_int)secpercyl);
-	ncg = howmany(ncyl, cpg);
+	ncg = howmany(ncyl, cylspg);
 	/*
 	 * Iterate a few times to allow for ipg depending on itself.
 	 */
@@ -1113,8 +1127,8 @@
 	for (i = 0; i < 10; i++) {
 		usedb = (sblock.fs_iblkno + ipg / INOPF(&sblock))
 			* NSPF(&sblock) * (off_t)sectorsize;
-		new_ipg = (cpg * (quad_t)bpcg - usedb) / density * fssize
-			  / ncg / secpercyl / cpg;
+		new_ipg = (cylspg * (quad_t)bpcg - usedb) / density * fssize
+			  / ncg / secpercyl / cylspg;
 		new_ipg = roundup(new_ipg, INOPB(&sblock));
 		if (new_ipg == ipg)
 			break;
@@ -1128,9 +1142,9 @@
  * Allocate an inode on the disk
  */
 void
-iput(register struct dinode *ip, register ino_t ino)
+iput(struct dinode *ip, ino_t ino)
 {
-	struct dinode buf[MAXINOPB];
+	struct dinode inobuf[MAXINOPB];
 	daddr_t d;
 	int c;
 
@@ -1150,14 +1164,14 @@
 	    (char *)&acg);
 	sblock.fs_cstotal.cs_nifree--;
 	fscs[0].cs_nifree--;
-	if (ino >= sblock.fs_ipg * sblock.fs_ncg) {
+	if (ino >= (uint32_t)sblock.fs_ipg * (uint32_t)sblock.fs_ncg) {
 		printf("fsinit: inode value out of range (%d).\n", ino);
 		exit(32);
 	}
 	d = fsbtodb(&sblock, ino_to_fsba(&sblock, ino));
-	rdfs(d, sblock.fs_bsize, (char *)buf);
-	buf[ino_to_fsbo(&sblock, ino)] = *ip;
-	wtfs(d, sblock.fs_bsize, (char *)buf);
+	rdfs(d, sblock.fs_bsize, (char *)inobuf);
+	inobuf[ino_to_fsbo(&sblock, ino)] = *ip;
+	wtfs(d, sblock.fs_bsize, (char *)inobuf);
 }
 
 /*
@@ -1167,7 +1181,7 @@
  * parent forked the child otherwise).
  */
 void
-parentready(void)
+parentready(__unused int signo)
 {
   	parentready_signalled = 1;
 }
@@ -1178,7 +1192,7 @@
  * We have to wait until the mount has actually completed!
  */
 void
-started(void)
+started(__unused int signo)
 {
 	int retry = 100;	/* 10 seconds, 100ms */
 
@@ -1207,7 +1221,7 @@
  * Replace libc function with one suited to our needs.
  */
 caddr_t
-malloc(register u_long size)
+malloc(u_long size)
 {
 	char *base, *i;
 	static u_long pgsz;
@@ -1306,7 +1320,7 @@
 
 	pgsz = getpagesize() - 1;
 	dstart = ((u_long)&etext) &~ pgsz;
-	freestart = ((u_long)(sbrk(0) + pgsz) &~ pgsz);
+	freestart = ((u_long)((char *)sbrk(0) + pgsz) &~ pgsz);
 	if (getrlimit(RLIMIT_DATA, &rlp) < 0)
 		warn("getrlimit");
 	memused = freestart - dstart;
Index: sbin/newfs/newfs.c
===================================================================
RCS file: /home/dcvs/src/sbin/newfs/newfs.c,v
retrieving revision 1.11
diff -u -r1.11 newfs.c
--- sbin/newfs/newfs.c	18 Dec 2004 21:43:39 -0000	1.11
+++ sbin/newfs/newfs.c	2 Jan 2005 19:38:08 -0000
@@ -77,7 +77,7 @@
 struct mntopt mopts[] = {
 	MOPT_STDOPTS,
 	MOPT_ASYNC,
-	{ NULL },
+	MOPT_NULL,
 };
 
 void	fatal(const char *fmt, ...);
@@ -211,16 +211,16 @@
 int
 main(int argc, char **argv)
 {
-	register int ch;
-	register struct partition *pp;
-	register struct disklabel *lp;
+	int ch;
+	struct partition *pp;
+	struct disklabel *lp;
 	struct disklabel mfsfakelabel;
-	struct disklabel *getdisklabel();
 	struct partition oldpartition;
 	struct stat st;
 	struct statfs *mp;
 	int fsi = 0, fso, len, n, vflag;
-	char *cp = NULL, *s1, *s2, *special, *opstring;
+	char *cp = NULL, *s1, *s2, *special;
+	const char *opstring;
 #ifdef MFS
 	struct vfsconf vfc;
 	int error;
@@ -485,7 +485,7 @@
 havelabel:
 	if (fssize == 0)
 		fssize = pp->p_size;
-	if (fssize > pp->p_size && !mfs)
+	if ((uint32_t)fssize > pp->p_size && !mfs)
 	       fatal("%s: maximum file system size on the `%c' partition is %d",
 			argv[0], *cp, pp->p_size);
 	if (rpm == 0) {
@@ -560,7 +560,7 @@
 	 * case (4096 sectors per cylinder) is intended to disagree
 	 * with the disklabel.
 	 */
-	if (t_or_u_flag && secpercyl != lp->d_secpercyl)
+	if (t_or_u_flag && (uint32_t)secpercyl != lp->d_secpercyl)
 		fprintf(stderr, "%s (%d) %s (%lu)\n",
 			"Warning: calculated sectors per cylinder", secpercyl,
 			"disagrees with disk label", (u_long)lp->d_secpercyl);
@@ -663,7 +663,7 @@
 #ifdef MFS
 
 static void
-mfsintr(int signo)
+mfsintr(__unused int signo)
 {
 	if (filename)
 		munmap(membase, fssize * sectorsize);
@@ -686,7 +686,7 @@
 	if (ioctl(fd, DIOCGDINFO, (char *)&lab) < 0) {
 #ifdef COMPAT
 		if (disktype) {
-			struct disklabel *lp, *getdiskbyname();
+			struct disklabel *lp;
 
 			unlabeled++;
 			lp = getdiskbyname(disktype);
Index: sbin/mount/mntopts.h
===================================================================
RCS file: /home/dcvs/src/sbin/mount/mntopts.h,v
retrieving revision 1.3
diff -u -r1.3 mntopts.h
--- sbin/mount/mntopts.h	1 Nov 2003 17:16:00 -0000	1.3
+++ sbin/mount/mntopts.h	2 Jan 2005 02:17:33 -0000
@@ -64,6 +64,9 @@
 #define MOPT_RO			{ "ro",		0, MNT_RDONLY, 0 }
 #define MOPT_RW			{ "rw",		1, MNT_RDONLY, 0 }
 
+/* NULL option; used to terminate arrays of options */
+#define MOPT_NULL		{ NULL,         0, 0, 0 }
+
 /* This is parsed by mount(8), but is ignored by specific mount_*(8)s. */
 #define MOPT_AUTO		{ "auto",	0, 0, 0 }
 




More information about the Submit mailing list