[PATCH 00/14] xfsprogs: varius & sundry fixes for coverity defects

[PATCH 00/14] xfsprogs: varius & sundry fixes for coverity defects

[Eric Sandeen]

This series knocks out 30 or so coverity defects.

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

[PATCH 01/14] xfsprogs: fix various fd leaks

[Eric Sandeen]

Coverity spotted these; several paths where we don't
close fds when we return.

Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---
 fsr/xfs_fsr.c      |   10 +++++++++-
 io/readdir.c       |    1 +
 libhandle/handle.c |    1 +
 mkfs/proto.c       |   12 +++++++++---
 quota/quot.c       |    1 +
 rtcp/xfs_rtcp.c    |    2 ++
 6 files changed, 23 insertions(+), 4 deletions(-)

diff --git a/fsr/xfs_fsr.c b/fsr/xfs_fsr.c
index 6f00b41..6b5d260 100644
--- a/fsr/xfs_fsr.c
+++ b/fsr/xfs_fsr.c
@@ -706,6 +706,7 @@ fsrfs(char *mntdir, xfs_ino_t startino, int targetrange)
 	if (xfs_getgeom(fsfd, &fsgeom) < 0 ) {
 		fsrprintf(_("Skipping %s: could not get XFS geometry\n"),
 			  mntdir);
+		close(fsfd);
 		return -1;
 	}
 
@@ -1346,6 +1347,8 @@ packfile(char *fname, char *tname, int fd,
 	if (lseek64(tfd, 0, SEEK_SET)) {
 		fsrprintf(_("Couldn't rewind on temporary file\n"));
 		close(tfd);
+		if (ffd != -1)
+			close(ffd);
 		free(fbuf);
 		return -1;
 	}
@@ -1359,6 +1362,8 @@ packfile(char *fname, char *tname, int fd,
 			fsrprintf(_("No improvement will be made (skipping): %s\n"), fname);
 		free(fbuf);
 		close(tfd);
+		if (ffd != -1)
+			close(ffd);
 		return 1; /* no change/no error */
 	}
 
@@ -1432,6 +1437,8 @@ packfile(char *fname, char *tname, int fd,
 				}
 				free(fbuf);
 				close(tfd);
+				if (ffd != -1)
+					close(ffd);
 				return -1;
 			}
 			if (nfrags) {
@@ -1446,7 +1453,8 @@ packfile(char *fname, char *tname, int fd,
 		}
 	}
 	ftruncate64(tfd, statp->bs_size);
-	if (ffd > 0) close(ffd);
+	if (ffd != -1)
+		close(ffd);
 	fsync(tfd);
 
 	free(fbuf);
diff --git a/io/readdir.c b/io/readdir.c
index 822818a..7133204 100644
--- a/io/readdir.c
+++ b/io/readdir.c
@@ -171,6 +171,7 @@ readdir_f(
 	gettimeofday(&t2, NULL);
 
 	closedir(dir);
+	close(dfd);
 
 	t2 = tsub(t2, t1);
 	timestr(&t2, ts, sizeof(ts), 0);
diff --git a/libhandle/handle.c b/libhandle/handle.c
index 9a232fa..9f81483 100644
--- a/libhandle/handle.c
+++ b/libhandle/handle.c
@@ -97,6 +97,7 @@ path_to_fshandle(
 		/* new filesystem. add it to the cache */
 		fdhp = malloc(sizeof(struct fdhash));
 		if (fdhp == NULL) {
+			close(fd);
 			errno = ENOMEM;
 			return -1;
 		}
diff --git a/mkfs/proto.c b/mkfs/proto.c
index 4d3680d..b7e0761 100644
--- a/mkfs/proto.c
+++ b/mkfs/proto.c
@@ -59,18 +59,19 @@ setup_proto(
 	if ((fd = open(fname, O_RDONLY)) < 0 || (size = filesize(fd)) < 0) {
 		fprintf(stderr, _("%s: failed to open %s: %s\n"),
 			progname, fname, strerror(errno));
-		exit(1);
+		goto out_fail;
 	}
+
 	buf = malloc(size + 1);
 	if (read(fd, buf, size) < size) {
 		fprintf(stderr, _("%s: read failed on %s: %s\n"),
 			progname, fname, strerror(errno));
-		exit(1);
+		goto out_fail;
 	}
 	if (buf[size - 1] != '\n') {
 		fprintf(stderr, _("%s: proto file %s premature EOF\n"),
 			progname, fname);
-		exit(1);
+		goto out_fail;
 	}
 	buf[size] = '\0';
 	/*
@@ -79,7 +80,12 @@ setup_proto(
 	(void)getstr(&buf);	/* boot image name */
 	(void)getnum(&buf);	/* block count */
 	(void)getnum(&buf);	/* inode count */
+	close(fd);
 	return buf;
+
+out_fail:
+	close(fd);
+	exit(1);
 }
 
 static long
diff --git a/quota/quot.c b/quota/quot.c
index a7782b4..96d449e 100644
--- a/quota/quot.c
+++ b/quota/quot.c
@@ -165,6 +165,7 @@ quot_bulkstat_mount(
 	buf = (xfs_bstat_t *)calloc(NBSTAT, sizeof(xfs_bstat_t));
 	if (!buf) {
 		perror("calloc");
+		close(fsfd);
 		return;
 	}
 
diff --git a/rtcp/xfs_rtcp.c b/rtcp/xfs_rtcp.c
index f0b5936..b81b1e1 100644
--- a/rtcp/xfs_rtcp.c
+++ b/rtcp/xfs_rtcp.c
@@ -224,6 +224,7 @@ rtcp( char *source, char *target, int fextsize)
 		if ( !(fsxattr.fsx_xflags & XFS_XFLAG_REALTIME) ) {
 			fprintf(stderr, _("%s: %s is not a realtime file.\n"),
 				progname, tbuf);
+			close( tofd );
 			return( -1 );
 		}
 
@@ -234,6 +235,7 @@ rtcp( char *source, char *target, int fextsize)
 			fprintf(stderr, _("%s: %s file extent size is %d, "
 					"instead of %d.\n"),
 				progname, tbuf, fsxattr.fsx_extsize, fextsize);
+			close( tofd );
 			return( -1 );
 		}
 	}
-- 
1.7.1

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

[PATCH 02/14] xfsprogs: refactor fsrall_cleanup in xfs_fsr

[Eric Sandeen]

fsrall_cleanup leaked an fd in the non-timeout
case - but the logic was weird and tortured, refactor
it to make more sense and fix the fd leak as well.

Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---
 fsr/xfs_fsr.c |   24 ++++++++++++------------
 1 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/fsr/xfs_fsr.c b/fsr/xfs_fsr.c
index 6b5d260..3818f02 100644
--- a/fsr/xfs_fsr.c
+++ b/fsr/xfs_fsr.c
@@ -647,14 +647,19 @@ fsrall_cleanup(int timeout)
 	int ret;
 	char buf[SMBUFSZ];
 
-	/* record where we left off */
 	unlink(leftofffile);
-	fd = open(leftofffile, O_WRONLY|O_CREAT|O_EXCL, 0644);
-	if (fd == -1)
-		fsrprintf(_("open(%s) failed: %s\n"),
-		          leftofffile, strerror(errno));
-	else {
-		if (timeout) {
+
+	if (timeout) {
+		fsrprintf(_("%s startpass %d, endpass %d, time %d seconds\n"),
+			progname, startpass, fs->npass,
+			time(0) - endtime + howlong);
+
+		/* record where we left off */
+		fd = open(leftofffile, O_WRONLY|O_CREAT|O_EXCL, 0644);
+		if (fd == -1) {
+			fsrprintf(_("open(%s) failed: %s\n"),
+			          leftofffile, strerror(errno));
+		} else {
 			ret = sprintf(buf, "%s %d %llu\n", fs->dev,
 			        fs->npass, (unsigned long long)leftoffino);
 			if (write(fd, buf, ret) < strlen(buf))
@@ -663,11 +668,6 @@ fsrall_cleanup(int timeout)
 			close(fd);
 		}
 	}
-
-	if (timeout)
-		fsrprintf(_("%s startpass %d, endpass %d, time %d seconds\n"),
-			progname, startpass, fs->npass,
-			time(0) - endtime + howlong);
 }
 
 /*
-- 
1.7.1

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

[PATCH 03/14] xfsprogs: trivial buffer frees on error paths

[Eric Sandeen]

Lots of memory leaks on error paths etc, spotted by
coverity.  This patch rolls up the super-straightforward
fixes across xfsprogs.

Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---
 db/addr.c    |   15 +++++++--------
 db/check.c   |    4 +++-
 db/write.c   |    2 ++
 io/parent.c  |    2 ++
 mkfs/proto.c |    3 ++-
 5 files changed, 16 insertions(+), 10 deletions(-)

diff --git a/db/addr.c b/db/addr.c
index f74dd62..f9f760d 100644
--- a/db/addr.c
+++ b/db/addr.c
@@ -85,16 +85,14 @@ addr_f(
 	fl = flist_scan(argv[1]);
 	if (fl == NULL)
 		return 0;
-	if (!flist_parse(fld, fl, iocur_top->data, 0)) {
-		flist_free(fl);
-		return 0;
-	}
+	if (!flist_parse(fld, fl, iocur_top->data, 0))
+		goto out;
+	
 	flist_print(fl);
 	for (tfl = fl; tfl->child != NULL; tfl = tfl->child) {
 		if ((tfl->flags & FL_OKLOW) && tfl->low < tfl->high) {
 			dbprintf(_("array not allowed for addr command\n"));
-			flist_free(fl);
-			return 0;
+			goto out;
 		}
 	}
 	fld = tfl->fld;
@@ -103,7 +101,7 @@ addr_f(
 		next = inode_next_type();
 	if (next == TYP_NONE) {
 		dbprintf(_("no next type for field %s\n"), fld->name);
-		return 0;
+		goto out;
 	}
 	fa = &ftattrtab[fld->ftyp];
 	ASSERT(fa->ftyp == fld->ftyp);
@@ -111,9 +109,10 @@ addr_f(
 	if (adf == NULL) {
 		dbprintf(_("no addr function for field %s (type %s)\n"),
 			fld->name, fa->name);
-		return 0;
+		goto out;
 	}
 	(*adf)(iocur_top->data, tfl->offset, next);
+out:
 	flist_free(fl);
 	return 0;
 }
diff --git a/db/check.c b/db/check.c
index 4867698..baf7f9f 100644
--- a/db/check.c
+++ b/db/check.c
@@ -1136,7 +1136,7 @@ blocktrash_f(
 	}
 	if (blocks == 0) {
 		dbprintf(_("blocktrash: no matching blocks\n"));
-		return 0;
+		goto out;
 	}
 	if (!sopt)
 		dbprintf(_("blocktrash: seed %u\n"), seed);
@@ -1161,6 +1161,7 @@ blocktrash_f(
 			}
 		}
 	}
+out:
 	xfree(lentab);
 	return 0;
 }
@@ -1907,6 +1908,7 @@ ncheck_f(
 			break;
 		default:
 			dbprintf(_("bad option -%c for ncheck command\n"), c);
+			xfree(ilist);
 			return 0;
 		}
 	}
diff --git a/db/write.c b/db/write.c
index 7b34fc0..ca8bd0f 100644
--- a/db/write.c
+++ b/db/write.c
@@ -233,6 +233,7 @@ bwrite_lrot(
 	memcpy(hold_region, base, shift);
 	memcpy(base, base+shift, len-shift);
 	memcpy(base+(len-shift), hold_region, shift);
+	free(hold_region);
 }
 
 /* ARGSUSED */
@@ -265,6 +266,7 @@ bwrite_rrot(
 	memcpy(hold_region, base+(len-shift), shift);
 	memmove(base+shift, base, len-shift);
 	memcpy(base, hold_region, shift);
+	free(hold_region);
 }
 
 /* ARGSUSED */
diff --git a/io/parent.c b/io/parent.c
index 47faaa0..ca989e9 100644
--- a/io/parent.c
+++ b/io/parent.c
@@ -258,6 +258,8 @@ parent_check(void)
 	if (!bstatbuf || !parentbuf) {
 		fprintf(stderr, _("unable to allocate buffers: %s\n"),
 			strerror(errno));
+		free(bstatbuf);
+		free(parentbuf);
 		return 1;
 	}
 
diff --git a/mkfs/proto.c b/mkfs/proto.c
index b7e0761..95583c9 100644
--- a/mkfs/proto.c
+++ b/mkfs/proto.c
@@ -49,7 +49,7 @@ char *
 setup_proto(
 	char	*fname)
 {
-	char		*buf;
+	char		*buf = NULL;
 	static char	dflt[] = "d--755 0 0 $";
 	int		fd;
 	long		size;
@@ -85,6 +85,7 @@ setup_proto(
 
 out_fail:
 	close(fd);
+	free(buf);
 	exit(1);
 }
 
-- 
1.7.1

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH 03/14] xfsprogs: trivial buffer frees on error paths

[Brian Foster]

On Tue, Apr 08, 2014 at 06:24:53PM -0500, Eric Sandeen wrote:
> Lots of memory leaks on error paths etc, spotted by
> coverity.  This patch rolls up the super-straightforward
> fixes across xfsprogs.
> 
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> ---
>  db/addr.c    |   15 +++++++--------
>  db/check.c   |    4 +++-
>  db/write.c   |    2 ++
>  io/parent.c  |    2 ++
>  mkfs/proto.c |    3 ++-
>  5 files changed, 16 insertions(+), 10 deletions(-)
> 
> diff --git a/db/addr.c b/db/addr.c
> index f74dd62..f9f760d 100644
> --- a/db/addr.c
> +++ b/db/addr.c
> @@ -85,16 +85,14 @@ addr_f(
>  	fl = flist_scan(argv[1]);
>  	if (fl == NULL)
>  		return 0;
> -	if (!flist_parse(fld, fl, iocur_top->data, 0)) {
> -		flist_free(fl);
> -		return 0;
> -	}
> +	if (!flist_parse(fld, fl, iocur_top->data, 0))
> +		goto out;
> +	

You've got some trailing whitespace here (tab in the empty line).

Brian

>  	flist_print(fl);
>  	for (tfl = fl; tfl->child != NULL; tfl = tfl->child) {
>  		if ((tfl->flags & FL_OKLOW) && tfl->low < tfl->high) {
>  			dbprintf(_("array not allowed for addr command\n"));
> -			flist_free(fl);
> -			return 0;
> +			goto out;
>  		}
>  	}
>  	fld = tfl->fld;
> @@ -103,7 +101,7 @@ addr_f(
>  		next = inode_next_type();
>  	if (next == TYP_NONE) {
>  		dbprintf(_("no next type for field %s\n"), fld->name);
> -		return 0;
> +		goto out;
>  	}
>  	fa = &ftattrtab[fld->ftyp];
>  	ASSERT(fa->ftyp == fld->ftyp);
> @@ -111,9 +109,10 @@ addr_f(
>  	if (adf == NULL) {
>  		dbprintf(_("no addr function for field %s (type %s)\n"),
>  			fld->name, fa->name);
> -		return 0;
> +		goto out;
>  	}
>  	(*adf)(iocur_top->data, tfl->offset, next);
> +out:
>  	flist_free(fl);
>  	return 0;
>  }
> diff --git a/db/check.c b/db/check.c
> index 4867698..baf7f9f 100644
> --- a/db/check.c
> +++ b/db/check.c
> @@ -1136,7 +1136,7 @@ blocktrash_f(
>  	}
>  	if (blocks == 0) {
>  		dbprintf(_("blocktrash: no matching blocks\n"));
> -		return 0;
> +		goto out;
>  	}
>  	if (!sopt)
>  		dbprintf(_("blocktrash: seed %u\n"), seed);
> @@ -1161,6 +1161,7 @@ blocktrash_f(
>  			}
>  		}
>  	}
> +out:
>  	xfree(lentab);
>  	return 0;
>  }
> @@ -1907,6 +1908,7 @@ ncheck_f(
>  			break;
>  		default:
>  			dbprintf(_("bad option -%c for ncheck command\n"), c);
> +			xfree(ilist);
>  			return 0;
>  		}
>  	}
> diff --git a/db/write.c b/db/write.c
> index 7b34fc0..ca8bd0f 100644
> --- a/db/write.c
> +++ b/db/write.c
> @@ -233,6 +233,7 @@ bwrite_lrot(
>  	memcpy(hold_region, base, shift);
>  	memcpy(base, base+shift, len-shift);
>  	memcpy(base+(len-shift), hold_region, shift);
> +	free(hold_region);
>  }
>  
>  /* ARGSUSED */
> @@ -265,6 +266,7 @@ bwrite_rrot(
>  	memcpy(hold_region, base+(len-shift), shift);
>  	memmove(base+shift, base, len-shift);
>  	memcpy(base, hold_region, shift);
> +	free(hold_region);
>  }
>  
>  /* ARGSUSED */
> diff --git a/io/parent.c b/io/parent.c
> index 47faaa0..ca989e9 100644
> --- a/io/parent.c
> +++ b/io/parent.c
> @@ -258,6 +258,8 @@ parent_check(void)
>  	if (!bstatbuf || !parentbuf) {
>  		fprintf(stderr, _("unable to allocate buffers: %s\n"),
>  			strerror(errno));
> +		free(bstatbuf);
> +		free(parentbuf);
>  		return 1;
>  	}
>  
> diff --git a/mkfs/proto.c b/mkfs/proto.c
> index b7e0761..95583c9 100644
> --- a/mkfs/proto.c
> +++ b/mkfs/proto.c
> @@ -49,7 +49,7 @@ char *
>  setup_proto(
>  	char	*fname)
>  {
> -	char		*buf;
> +	char		*buf = NULL;
>  	static char	dflt[] = "d--755 0 0 $";
>  	int		fd;
>  	long		size;
> @@ -85,6 +85,7 @@ setup_proto(
>  
>  out_fail:
>  	close(fd);
> +	free(buf);
>  	exit(1);
>  }
>  
> -- 
> 1.7.1
> 
> _______________________________________________
> xfs mailing list
> xfs@oss.sgi.com
> http://oss.sgi.com/mailman/listinfo/xfs

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

[PATCH 03/14 V2] xfsprogs: trivial buffer frees on error paths

[Eric Sandeen]

Lots of memory leaks on error paths etc, spotted by
coverity.  This patch rolls up the super-straightforward
fixes across xfsprogs.

Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---

V2: Fix whitespace error

 db/addr.c    |   15 +++++++--------
 db/check.c   |    4 +++-
 db/write.c   |    2 ++
 io/parent.c  |    2 ++
 mkfs/proto.c |    3 ++-
 5 files changed, 16 insertions(+), 10 deletions(-)

diff --git a/db/addr.c b/db/addr.c
index f74dd62..f9f760d 100644
--- a/db/addr.c
+++ b/db/addr.c
@@ -85,16 +85,14 @@ addr_f(
 	fl = flist_scan(argv[1]);
 	if (fl == NULL)
 		return 0;
-	if (!flist_parse(fld, fl, iocur_top->data, 0)) {
-		flist_free(fl);
-		return 0;
-	}
+	if (!flist_parse(fld, fl, iocur_top->data, 0))
+		goto out;
+
 	flist_print(fl);
 	for (tfl = fl; tfl->child != NULL; tfl = tfl->child) {
 		if ((tfl->flags & FL_OKLOW) && tfl->low < tfl->high) {
 			dbprintf(_("array not allowed for addr command\n"));
-			flist_free(fl);
-			return 0;
+			goto out;
 		}
 	}
 	fld = tfl->fld;
@@ -103,7 +101,7 @@ addr_f(
 		next = inode_next_type();
 	if (next == TYP_NONE) {
 		dbprintf(_("no next type for field %s\n"), fld->name);
-		return 0;
+		goto out;
 	}
 	fa = &ftattrtab[fld->ftyp];
 	ASSERT(fa->ftyp == fld->ftyp);
@@ -111,9 +109,10 @@ addr_f(
 	if (adf == NULL) {
 		dbprintf(_("no addr function for field %s (type %s)\n"),
 			fld->name, fa->name);
-		return 0;
+		goto out;
 	}
 	(*adf)(iocur_top->data, tfl->offset, next);
+out:
 	flist_free(fl);
 	return 0;
 }
diff --git a/db/check.c b/db/check.c
index 4867698..baf7f9f 100644
--- a/db/check.c
+++ b/db/check.c
@@ -1136,7 +1136,7 @@ blocktrash_f(
 	}
 	if (blocks == 0) {
 		dbprintf(_("blocktrash: no matching blocks\n"));
-		return 0;
+		goto out;
 	}
 	if (!sopt)
 		dbprintf(_("blocktrash: seed %u\n"), seed);
@@ -1161,6 +1161,7 @@ blocktrash_f(
 			}
 		}
 	}
+out:
 	xfree(lentab);
 	return 0;
 }
@@ -1907,6 +1908,7 @@ ncheck_f(
 			break;
 		default:
 			dbprintf(_("bad option -%c for ncheck command\n"), c);
+			xfree(ilist);
 			return 0;
 		}
 	}
diff --git a/db/write.c b/db/write.c
index 7b34fc0..ca8bd0f 100644
--- a/db/write.c
+++ b/db/write.c
@@ -233,6 +233,7 @@ bwrite_lrot(
 	memcpy(hold_region, base, shift);
 	memcpy(base, base+shift, len-shift);
 	memcpy(base+(len-shift), hold_region, shift);
+	free(hold_region);
 }
 
 /* ARGSUSED */
@@ -265,6 +266,7 @@ bwrite_rrot(
 	memcpy(hold_region, base+(len-shift), shift);
 	memmove(base+shift, base, len-shift);
 	memcpy(base, hold_region, shift);
+	free(hold_region);
 }
 
 /* ARGSUSED */
diff --git a/io/parent.c b/io/parent.c
index 47faaa0..ca989e9 100644
--- a/io/parent.c
+++ b/io/parent.c
@@ -258,6 +258,8 @@ parent_check(void)
 	if (!bstatbuf || !parentbuf) {
 		fprintf(stderr, _("unable to allocate buffers: %s\n"),
 			strerror(errno));
+		free(bstatbuf);
+		free(parentbuf);
 		return 1;
 	}
 
diff --git a/mkfs/proto.c b/mkfs/proto.c
index b7e0761..95583c9 100644
--- a/mkfs/proto.c
+++ b/mkfs/proto.c
@@ -49,7 +49,7 @@ char *
 setup_proto(
 	char	*fname)
 {
-	char		*buf;
+	char		*buf = NULL;
 	static char	dflt[] = "d--755 0 0 $";
 	int		fd;
 	long		size;
@@ -85,6 +85,7 @@ setup_proto(
 
 out_fail:
 	close(fd);
+	free(buf);
 	exit(1);
 }
 
-- 1.7.1 _______________________________________________ xfs mailing list xfs@oss.sgi.com http://oss.sgi.com/mailman/listinfo/xfs

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

[PATCH 04/14] xfsprogs: fix memory leak in xlog_recover_add_to_trans

[Eric Sandeen]

Free the memory in error path of xlog_recover_add_to_trans().
Normally this memory is freed in recovery pass2, but is leaked
in the error path.

Userspace version of kernel commits 519ccb8 & aaaae98

Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---
 libxlog/xfs_log_recover.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/libxlog/xfs_log_recover.c b/libxlog/xfs_log_recover.c
index 3f22921..d1fa535 100644
--- a/libxlog/xfs_log_recover.c
+++ b/libxlog/xfs_log_recover.c
@@ -1131,6 +1131,7 @@ xlog_recover_add_to_trans(
 		"bad number of regions (%d) in inode log format",
 				  in_f->ilf_size);
 			ASSERT(0);
+			kmem_free(ptr);
 			return XFS_ERROR(EIO);
 		}
 
-- 
1.7.1

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

[PATCH 05/14] libxfs: fix memory leak in xfs_dir2_node_removename

[Eric Sandeen]

Fix the leak of kernel memory in xfs_dir2_node_removename()
when xfs_dir2_leafn_remove() returns an error code.

Cross-port of kernel commit 3a8c9208

Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---
 libxfs/xfs_dir2_node.c |   26 +++++++++++++-------------
 1 files changed, 13 insertions(+), 13 deletions(-)

diff --git a/libxfs/xfs_dir2_node.c b/libxfs/xfs_dir2_node.c
index 3256756..3737e4e 100644
--- a/libxfs/xfs_dir2_node.c
+++ b/libxfs/xfs_dir2_node.c
@@ -2088,12 +2088,12 @@ xfs_dir2_node_lookup(
  */
 int						/* error */
 xfs_dir2_node_removename(
-	xfs_da_args_t		*args)		/* operation arguments */
+	struct xfs_da_args	*args)		/* operation arguments */
 {
-	xfs_da_state_blk_t	*blk;		/* leaf block */
+	struct xfs_da_state_blk	*blk;		/* leaf block */
 	int			error;		/* error return value */
 	int			rval;		/* operation return value */
-	xfs_da_state_t		*state;		/* btree cursor */
+	struct xfs_da_state	*state;		/* btree cursor */
 
 	trace_xfs_dir2_node_removename(args);
 
@@ -2105,19 +2105,18 @@ xfs_dir2_node_removename(
 	state->mp = args->dp->i_mount;
 	state->blocksize = state->mp->m_dirblksize;
 	state->node_ents = state->mp->m_dir_node_ents;
-	/*
-	 * Look up the entry we're deleting, set up the cursor.
-	 */
+
+	/* Look up the entry we're deleting, set up the cursor. */
 	error = xfs_da3_node_lookup_int(state, &rval);
 	if (error)
-		rval = error;
-	/*
-	 * Didn't find it, upper layer screwed up.
-	 */
+		goto out_free;
+
+	/* Didn't find it, upper layer screwed up. */
 	if (rval != EEXIST) {
-		xfs_da_state_free(state);
-		return rval;
+		error = rval;
+		goto out_free;
 	}
+
 	blk = &state->path.blk[state->path.active - 1];
 	ASSERT(blk->magic == XFS_DIR2_LEAFN_MAGIC);
 	ASSERT(state->extravalid);
@@ -2128,7 +2127,7 @@ xfs_dir2_node_removename(
 	error = xfs_dir2_leafn_remove(args, blk->bp, blk->index,
 		&state->extrablk, &rval);
 	if (error)
-		return error;
+		goto out_free;
 	/*
 	 * Fix the hash values up the btree.
 	 */
@@ -2143,6 +2142,7 @@ xfs_dir2_node_removename(
 	 */
 	if (!error)
 		error = xfs_dir2_node_to_leaf(state);
+out_free:
 	xfs_da_state_free(state);
 	return error;
 }
-- 
1.7.1

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

[PATCH 06/14] xfs_quota: fix memory leak in quota_group_type() error path

[Eric Sandeen]

quota_group_type has some rather contorted logic that's
been around since 2005.

In the (!name) case, if any of the 3 calls setting up ngroups fails,
we fall back to using just one group.

However, if it's the getgroups() call that fails, we overwrite
the allocated gid ptr with &gid, thus leaking that allocated
memory.  Worse, we set "dofree" to 1, so will free non-allocated
local var gid.  And that last else case is redundant; if we get there,
gids is guaranteed to be non-null.

Refactor it a bit to be more clear (I hope) and correct.

Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---
 quota/quota.c |   20 ++++++++++++--------
 1 files changed, 12 insertions(+), 8 deletions(-)

diff --git a/quota/quota.c b/quota/quota.c
index 7e52ad2..367da8c 100644
--- a/quota/quota.c
+++ b/quota/quota.c
@@ -289,15 +289,19 @@ quota_group_type(
 		}
 		gids = &gid;
 		ngroups = 1;
-	} else if ( ((ngroups = sysconf(_SC_NGROUPS_MAX)) < 0) ||
-		    ((gids = malloc(ngroups * sizeof(gid_t))) == NULL) ||
-		    ((ngroups = getgroups(ngroups, gids)) < 0)) {
-		dofree = (gids != NULL);
-		gid = getgid();
-		gids = &gid;
-		ngroups = 1;
 	} else {
-		dofree = (gids != NULL);
+		if ( ((ngroups = sysconf(_SC_NGROUPS_MAX)) < 0) ||
+		     ((gids = malloc(ngroups * sizeof(gid_t))) == NULL) ||
+		     ((ngroups = getgroups(ngroups, gids)) < 0)) {
+			/* something failed.  Fall back to 1 group */
+			free(gids);
+			gid = getgid();
+			gids = &gid;
+			ngroups = 1;
+		} else {
+			/* It all worked, and we allocated memory */
+			dofree = 1;
+		}
 	}
 
 	for (i = 0; i < ngroups; i++, name = NULL) {
-- 
1.7.1

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

[PATCH 07/14] xfs_logprint: fix leak in error path of xlog_print_record()

[Eric Sandeen]

In 2 error paths we returned without freeing the allocated buf.
Collapse them into a compound test & free buf on the way out.

Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---
 logprint/log_misc.c |   11 ++++++-----
 1 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/logprint/log_misc.c b/logprint/log_misc.c
index 52f1e85..a022df4 100644
--- a/logprint/log_misc.c
+++ b/logprint/log_misc.c
@@ -942,11 +942,12 @@ xlog_print_record(
 	     */
 	    if (be32_to_cpu(rhead->h_cycle) !=
 			be32_to_cpu(*(__be32 *)ptr)) {
-		if (*read_type == FULL_READ)
-		    return -1;
-		else if (be32_to_cpu(rhead->h_cycle) + 1 !=
-			be32_to_cpu(*(__be32 *)ptr))
-		    return -1;
+		if ((*read_type == FULL_READ) ||
+		    (be32_to_cpu(rhead->h_cycle) + 1 !=
+				be32_to_cpu(*(__be32 *)ptr))) {
+			free(buf);
+			return -1;
+		}
 	    }
 	}
 
-- 
1.7.1

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

[PATCH 08/14] xfsprogs: free resources in libxfs_alloc_file_space error paths

[Eric Sandeen]

The bmap freelist & transaction pointer weren't
being freed in libxfs_alloc_file_space error paths;
more or less copy the error handling that exists
in kernelspace to resolve this.

Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---
 libxfs/util.c |   20 +++++++++++++++++---
 1 files changed, 17 insertions(+), 3 deletions(-)

diff --git a/libxfs/util.c b/libxfs/util.c
index 1b05540..f1aa4c6 100644
--- a/libxfs/util.c
+++ b/libxfs/util.c
@@ -582,8 +582,17 @@ libxfs_alloc_file_space(
 		resblks = (uint)XFS_DIOSTRAT_SPACE_RES(mp, datablocks);
 		error = xfs_trans_reserve(tp, &M_RES(mp)->tr_write,
 					  resblks, 0);
-		if (error)
+		/*
+		 * Check for running out of space
+		 */
+		if (error) {
+			/*
+			 * Free the transaction structure.
+			 */
+			ASSERT(error == ENOSPC); /* XXX ERS? */
+			xfs_trans_cancel(tp, 0);
 			break;
+		}
 		xfs_trans_ijoin(tp, ip, 0);
 		xfs_trans_ihold(tp, ip);
 
@@ -593,12 +602,12 @@ libxfs_alloc_file_space(
 				&reccount, &free_list);
 
 		if (error)
-			break;
+			goto error0;
 
 		/* complete the transaction */
 		error = xfs_bmap_finish(&tp, &free_list, &committed);
 		if (error)
-			break;
+			goto error0;
 
 		error = xfs_trans_commit(tp, 0);
 		if (error)
@@ -612,6 +621,11 @@ libxfs_alloc_file_space(
 		allocatesize_fsb -= allocated_fsb;
 	}
 	return error;
+
+error0:	/* Cancel bmap, unlock inode, cancel trans */
+	xfs_bmap_cancel(&free_list);
+	xfs_trans_cancel(tp, 0);
+	return error;
 }
 
 unsigned int
-- 
1.7.1

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH 08/14] xfsprogs: free resources in libxfs_alloc_file_space error paths

[Brian Foster]

On Tue, Apr 08, 2014 at 06:24:58PM -0500, Eric Sandeen wrote:
> The bmap freelist & transaction pointer weren't
> being freed in libxfs_alloc_file_space error paths;
> more or less copy the error handling that exists
> in kernelspace to resolve this.
> 
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> ---
>  libxfs/util.c |   20 +++++++++++++++++---
>  1 files changed, 17 insertions(+), 3 deletions(-)
> 
> diff --git a/libxfs/util.c b/libxfs/util.c
> index 1b05540..f1aa4c6 100644
> --- a/libxfs/util.c
> +++ b/libxfs/util.c
> @@ -582,8 +582,17 @@ libxfs_alloc_file_space(
>  		resblks = (uint)XFS_DIOSTRAT_SPACE_RES(mp, datablocks);
>  		error = xfs_trans_reserve(tp, &M_RES(mp)->tr_write,
>  					  resblks, 0);
> -		if (error)
> +		/*
> +		 * Check for running out of space
> +		 */
> +		if (error) {
> +			/*
> +			 * Free the transaction structure.
> +			 */
> +			ASSERT(error == ENOSPC); /* XXX ERS? */

XXX ?

> +			xfs_trans_cancel(tp, 0);
>  			break;
> +		}
>  		xfs_trans_ijoin(tp, ip, 0);
>  		xfs_trans_ihold(tp, ip);
>  
> @@ -593,12 +602,12 @@ libxfs_alloc_file_space(
>  				&reccount, &free_list);
>  
>  		if (error)
> -			break;
> +			goto error0;
>  
>  		/* complete the transaction */
>  		error = xfs_bmap_finish(&tp, &free_list, &committed);
>  		if (error)
> -			break;
> +			goto error0;
>  
>  		error = xfs_trans_commit(tp, 0);
>  		if (error)
> @@ -612,6 +621,11 @@ libxfs_alloc_file_space(
>  		allocatesize_fsb -= allocated_fsb;
>  	}
>  	return error;
> +
> +error0:	/* Cancel bmap, unlock inode, cancel trans */

Nit, but since it appears we don't lock the inode, the comment is wrong.

Brian

> +	xfs_bmap_cancel(&free_list);
> +	xfs_trans_cancel(tp, 0);
> +	return error;
>  }
>  
>  unsigned int
> -- 
> 1.7.1
> 
> _______________________________________________
> xfs mailing list
> xfs@oss.sgi.com
> http://oss.sgi.com/mailman/listinfo/xfs

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

[PATCH 08/14 V2] xfsprogs: free resources in libxfs_alloc_file_space error paths

[Eric Sandeen]

The bmap freelist & transaction pointer weren't
being freed in libxfs_alloc_file_space error paths;
more or less copy the error handling that exists
in kernelspace to resolve this.

Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---

V2: Fix comment, remove XXX :)

Although:  That ASSERT, while matching kernelspace, seems
weird.  We *can* get other errors out of xfs_trans_reserve.
EIO etc... What's the assert all about?

 libxfs/util.c |   20 +++++++++++++++++---
 1 files changed, 17 insertions(+), 3 deletions(-)

diff --git a/libxfs/util.c b/libxfs/util.c
index 1b05540..f1aa4c6 100644
--- a/libxfs/util.c
+++ b/libxfs/util.c
@@ -582,8 +582,17 @@ libxfs_alloc_file_space(
 		resblks = (uint)XFS_DIOSTRAT_SPACE_RES(mp, datablocks);
 		error = xfs_trans_reserve(tp, &M_RES(mp)->tr_write,
 					  resblks, 0);
-		if (error)
+		/*
+		 * Check for running out of space
+		 */
+		if (error) {
+			/*
+			 * Free the transaction structure.
+			 */
+			ASSERT(error == ENOSPC);
+			xfs_trans_cancel(tp, 0);
 			break;
+		}
 		xfs_trans_ijoin(tp, ip, 0);
 		xfs_trans_ihold(tp, ip);
 
@@ -593,12 +602,12 @@ libxfs_alloc_file_space(
 				&reccount, &free_list);
 
 		if (error)
-			break;
+			goto error0;
 
 		/* complete the transaction */
 		error = xfs_bmap_finish(&tp, &free_list, &committed);
 		if (error)
-			break;
+			goto error0;
 
 		error = xfs_trans_commit(tp, 0);
 		if (error)
@@ -612,6 +621,11 @@ libxfs_alloc_file_space(
 		allocatesize_fsb -= allocated_fsb;
 	}
 	return error;
+
+error0:	/* Cancel bmap, cancel trans */
+	xfs_bmap_cancel(&free_list);
+	xfs_trans_cancel(tp, 0);
+	return error;
 }
 
 unsigned int


_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

[PATCH 09/14] xfsprogs: annotate a case fallthrough in libxfs_ialloc

[Eric Sandeen]

This is all working as intended, but add a comment to
make it more obvious to readers and static code
checkers.

Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---
 libxfs/util.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/libxfs/util.c b/libxfs/util.c
index f1aa4c6..1e48079 100644
--- a/libxfs/util.c
+++ b/libxfs/util.c
@@ -265,6 +265,7 @@ libxfs_ialloc(
 	case S_IFSOCK:
 		/* doesn't make sense to set an rdev for these */
 		rdev = 0;
+		/* FALLTHROUGH */
 	case S_IFCHR:
 	case S_IFBLK:
 		ip->i_d.di_format = XFS_DINODE_FMT_DEV;
-- 
1.7.1

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

[PATCH 10/14] xfsprogs: fix too-large memset value in xfs_db's attr code

[Eric Sandeen]

	memset(value, 0xfeedface, valuelen);

seemed to be trying to fill a new attr with some
recognizeable magic, but of course memset can
only set a single byte; switch this to use 'v'

Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---
 db/attrset.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/db/attrset.c b/db/attrset.c
index 75be72e..762b3bf 100644
--- a/db/attrset.c
+++ b/db/attrset.c
@@ -146,7 +146,7 @@ attr_set_f(
 			dbprintf(_("cannot allocate buffer (%d)\n"), valuelen);
 			goto out;
 		}
-		memset(value, 0xfeedface, valuelen);
+		memset(value, 'v', valuelen);
 	} else {
 		value = NULL;
 	}
-- 
1.7.1

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

[PATCH 11/14] xfs_quota: remove impossible tests in printpath

[Eric Sandeen]

printpath() had some cut & paste tests of "c" - but
nothing had set it yet other than c=0, so testing it
is pointless.  Just remove tests for non-zero "c"
until we might have set it to something interesting.

Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---
 quota/path.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/quota/path.c b/quota/path.c
index e9a6834..47fdd93 100644
--- a/quota/path.c
+++ b/quota/path.c
@@ -54,9 +54,9 @@ printpath(
 		c = 0;
 		printf(" (");
 		if (qstat.qs_flags & XFS_QUOTA_UDQ_ENFD)
-			c = printf("%suquota", c ? ", " : "");
+			c = printf("uquota");
 		else if (qstat.qs_flags & XFS_QUOTA_UDQ_ACCT)
-			c = printf("%suqnoenforce", c ? ", " : "");
+			c = printf("uqnoenforce");
 		if (qstat.qs_flags & XFS_QUOTA_GDQ_ENFD)
 			c = printf("%sgquota", c ? ", " : "");
 		else if (qstat.qs_flags & XFS_QUOTA_GDQ_ACCT)
-- 
1.7.1

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

[PATCH 12/14] xfs_repair: address never-true tests in repair/bmap.c on 64 bit

[Eric Sandeen]

The test "if (new_naexts > BLKMAP_NEXTS_MAX)" is never true
on a 64-bit platform; new_naexts is an int, and BLKMAP_NEXTS_MAX
is INT_MAX.  So just wrap the whole thing in the #ifdef.

Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---
 repair/bmap.c |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/repair/bmap.c b/repair/bmap.c
index 85d66dc..b81efb9 100644
--- a/repair/bmap.c
+++ b/repair/bmap.c
@@ -47,16 +47,16 @@ blkmap_alloc(
 	if (nex < 1)
 		nex = 1;
 
+#if (BITS_PER_LONG == 32)	/* on 64-bit platformsethis is never true */
 	if (nex > BLKMAP_NEXTS_MAX) {
-#if (BITS_PER_LONG == 32)
 		do_warn(
 	_("Number of extents requested in blkmap_alloc (%d) overflows 32 bits.\n"
 	  "If this is not a corruption, then you will need a 64 bit system\n"
 	  "to repair this filesystem.\n"),
 			nex);
-#endif
 		return NULL;
 	}
+#endif
 
 	key = whichfork ? ablkmap_key : dblkmap_key;
 	blkmap = pthread_getspecific(key);
@@ -267,15 +267,15 @@ blkmap_grow(
 		ASSERT(pthread_getspecific(key) == blkmap);
 	}
 
+#if (BITS_PER_LONG == 32)	/* on 64-bit platforms this is never true */
 	if (new_naexts > BLKMAP_NEXTS_MAX) {
-#if (BITS_PER_LONG == 32)
 		do_error(
 	_("Number of extents requested in blkmap_grow (%d) overflows 32 bits.\n"
 	  "You need a 64 bit system to repair this filesystem.\n"),
 			new_naexts);
-#endif
 		return NULL;
 	}
+#endif
 	if (new_naexts <= 0) {
 		do_error(
 	_("Number of extents requested in blkmap_grow (%d) overflowed the\n"
-- 
1.7.1

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH 12/14] xfs_repair: address never-true tests in repair/bmap.c on 64 bit

[Mark Tinguely]

On 04/08/14 18:25, Eric Sandeen wrote:
> The test "if (new_naexts > BLKMAP_NEXTS_MAX)" is never true
> on a 64-bit platform; new_naexts is an int, and BLKMAP_NEXTS_MAX
> is INT_MAX.  So just wrap the whole thing in the #ifdef.
>
> Signed-off-by: Eric Sandeen<sandeen@redhat.com>
> ---
>   repair/bmap.c |    8 ++++----
>   1 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/repair/bmap.c b/repair/bmap.c
> index 85d66dc..b81efb9 100644
> --- a/repair/bmap.c
> +++ b/repair/bmap.c
> @@ -47,16 +47,16 @@ blkmap_alloc(
>   	if (nex < 1)
>   		nex = 1;
>
> +#if (BITS_PER_LONG == 32)	/* on 64-bit platformsethis is never true */
                                              platforms this ..,
>   	if (nex > BLKMAP_NEXTS_MAX) {
> -#if (BITS_PER_LONG == 32)
>   		do_warn(
>   	_("Number of extents requested in blkmap_alloc (%d) overflows 32 bits.\n"
>   	  "If this is not a corruption, then you will need a 64 bit system\n"
>   	  "to repair this filesystem.\n"),
>   			nex);
> -#endif
>   		return NULL;
>   	}
> +#endif

Looks good. The other patches look good too.

Reviewed-by: Mark Tinguely <tinguely@sgi.com>

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

[PATCH 12/14 V2] xfs_repair: address never-true tests in repair/bmap.c on 64 bit

[Eric Sandeen]

The test "if (new_naexts > BLKMAP_NEXTS_MAX)" is never true
on a 64-bit platform; new_naexts is an int, and BLKMAP_NEXTS_MAX
is INT_MAX.  So just wrap the whole thing in the #ifdef.

Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---

V2: Fix typo in comment in first hunk

 repair/bmap.c |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/repair/bmap.c b/repair/bmap.c
index 85d66dc..b81efb9 100644
--- a/repair/bmap.c
+++ b/repair/bmap.c
@@ -47,16 +47,16 @@ blkmap_alloc(
 	if (nex < 1)
 		nex = 1;
 
+#if (BITS_PER_LONG == 32)	/* on 64-bit platforms this is never true */
 	if (nex > BLKMAP_NEXTS_MAX) {
-#if (BITS_PER_LONG == 32)
 		do_warn(
 	_("Number of extents requested in blkmap_alloc (%d) overflows 32 bits.\n"
 	  "If this is not a corruption, then you will need a 64 bit system\n"
 	  "to repair this filesystem.\n"),
 			nex);
-#endif
 		return NULL;
 	}
+#endif
 
 	key = whichfork ? ablkmap_key : dblkmap_key;
 	blkmap = pthread_getspecific(key);
@@ -267,15 +267,15 @@ blkmap_grow(
 		ASSERT(pthread_getspecific(key) == blkmap);
 	}
 
+#if (BITS_PER_LONG == 32)	/* on 64-bit platforms this is never true */
 	if (new_naexts > BLKMAP_NEXTS_MAX) {
-#if (BITS_PER_LONG == 32)
 		do_error(
 	_("Number of extents requested in blkmap_grow (%d) overflows 32 bits.\n"
 	  "You need a 64 bit system to repair this filesystem.\n"),
 			new_naexts);
-#endif
 		return NULL;
 	}
+#endif
 	if (new_naexts <= 0) {
 		do_error(
 	_("Number of extents requested in blkmap_grow (%d) overflowed the\n"


_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

[PATCH 13/14] mkfs: catch unknown format in protofile parsing

[Eric Sandeen]

As the code stands today we can't get an unknown format in the
last case statement, but Coverity warns that if we ever do, we'll
use an uninitialized "ip" in the call to libxfs_trans_log_inode().

Adding a default: case to catch unknown formats is defensive and
makes the checker happy.

Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---
 mkfs/proto.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/mkfs/proto.c b/mkfs/proto.c
index 95583c9..308325b 100644
--- a/mkfs/proto.c
+++ b/mkfs/proto.c
@@ -591,6 +591,9 @@ parseproto(
 		}
 		libxfs_iput(ip, 0);
 		return;
+	default:
+		ASSERT(0);
+		fail(_("Unknown format"), EINVAL);
 	}
 	libxfs_trans_log_inode(tp, ip, flags);
 	error = libxfs_bmap_finish(&tp, &flist, &committed);
-- 
1.7.1

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

[PATCH 14/14] xfs_db: don't use invalid index in ring_f

[Eric Sandeen]

ring_f() tests for an invalid index which would overrun the
iocur_ring[] array and warns, but then uses it anyway.

Return immediately if it's out of bounds.

Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---
 db/io.c |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/db/io.c b/db/io.c
index 9a787c8..6aba54e 100644
--- a/db/io.c
+++ b/db/io.c
@@ -358,8 +358,10 @@ ring_f(
 	}
 
 	index = (int)strtoul(argv[1], NULL, 0);
-	if (index < 0 || index >= RING_ENTRIES)
+	if (index < 0 || index >= RING_ENTRIES) {
 		dbprintf(_("invalid entry: %d\n"), index);
+		return 0;
+	}
 
 	ring_current = index;
 
-- 
1.7.1

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

[PATCH 15/14] xfs_io: free memory on error path exit from bmap_f()

[Eric Sandeen]

If realloc() fails, the memory it was trying to realloc remains;
free it on the error path.

Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---

diff --git a/io/bmap.c b/io/bmap.c
index a78cbb1..8e5d75e 100644
--- a/io/bmap.c
+++ b/io/bmap.c
@@ -227,6 +227,7 @@ bmap_f(
 				fprintf(stderr,
 					_("%s: cannot realloc %d bytes\n"),
 					progname, (int)(map_size*sizeof(*map)));
+				free(map);
 				exitcode = 1;
 				return 0;
 			}


_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH 15/14] xfs_io: free memory on error path exit from bmap_f()

[Mark Tinguely]

On 04/09/14 17:18, Eric Sandeen wrote:
> If realloc() fails, the memory it was trying to realloc remains;
> free it on the error path.
>
> Signed-off-by: Eric Sandeen<sandeen@redhat.com>
> ---
>
> diff --git a/io/bmap.c b/io/bmap.c
> index a78cbb1..8e5d75e 100644
> --- a/io/bmap.c
> +++ b/io/bmap.c
> @@ -227,6 +227,7 @@ bmap_f(
>   				fprintf(stderr,
>   					_("%s: cannot realloc %d bytes\n"),
>   					progname, (int)(map_size*sizeof(*map)));
> +				free(map);
>   				exitcode = 1;
>   				return 0;
>   			}


Didn't it get into this block by comparing map to NULL?

--Mark.

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH 15/14] xfs_io: free memory on error path exit from bmap_f()

[Eric Sandeen]

On 4/10/14, 8:38 AM, Mark Tinguely wrote:
> On 04/09/14 17:18, Eric Sandeen wrote:
>> If realloc() fails, the memory it was trying to realloc remains;
>> free it on the error path.
>>
>> Signed-off-by: Eric Sandeen<sandeen@redhat.com>
>> ---
>>
>> diff --git a/io/bmap.c b/io/bmap.c
>> index a78cbb1..8e5d75e 100644
>> --- a/io/bmap.c
>> +++ b/io/bmap.c
>> @@ -227,6 +227,7 @@ bmap_f(
>>                   fprintf(stderr,
>>                       _("%s: cannot realloc %d bytes\n"),
>>                       progname, (int)(map_size*sizeof(*map)));
>> +                free(map);
>>                   exitcode = 1;
>>                   return 0;
>>               }
> 
> 
> Didn't it get into this block by comparing map to NULL?

Uh yes, yes we did.  Sorry, was getting a bit slap-happy by then.
I guess p = realloc(p) will always lose the memory if it fails,
w/o a temporary var.


Sorry about that!
-Eric

 
> --Mark.
> 

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH 00/14] xfsprogs: varius & sundry fixes for coverity defects

[Brian Foster]

On Tue, Apr 08, 2014 at 06:24:50PM -0500, Eric Sandeen wrote:
> This series knocks out 30 or so coverity defects.
> 

Noted a few nits... things that could probably be fixed up at commit
time if so desired. Otherwise, the series looks good to me. Thanks Eric.

Reviewed-by: Brian Foster <bfoster@redhat.com>

> _______________________________________________
> xfs mailing list
> xfs@oss.sgi.com
> http://oss.sgi.com/mailman/listinfo/xfs

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs