gfs2-utils: master - gfs2-utils: Fix up some errors reported by clang

Andrew Price andyp at fedoraproject.org
Mon Mar 3 18:44:16 UTC 2014


Gitweb:        http://git.fedorahosted.org/git/?p=gfs2-utils.git;a=commitdiff;h=57f0e1b70dbd1de7aca9b2bdc09d49d28c25d888
Commit:        57f0e1b70dbd1de7aca9b2bdc09d49d28c25d888
Parent:        4b5eb22347c722ca5a2a3db898e3babce8225d7d
Author:        Andrew Price <anprice at redhat.com>
AuthorDate:    Sat Feb 22 02:37:44 2014 +0000
Committer:     Andrew Price <anprice at redhat.com>
CommitterDate: Thu Feb 27 20:26:51 2014 +0000

gfs2-utils: Fix up some errors reported by clang

Clang's analyzer found some potential memory leaks, most of which in
reality could never manifest, but it highlighted some places where
malloc and memset were being used unnecessarily, so those have been
removed and replaced with calloc() or literal assignments.

There were also some cases of dead assignments and initialisations, and
uses of uninitialised variables which have also been fixed up.

Signed-off-by: Andrew Price <anprice at redhat.com>
---
 gfs2/edit/savemeta.c   |    7 ++---
 gfs2/fsck/inode_hash.c |    6 +---
 gfs2/fsck/main.c       |    2 +-
 gfs2/fsck/pass2.c      |   64 ++++++-----------------------------------------
 gfs2/fsck/pass3.c      |   22 ++--------------
 gfs2/fsck/rgrepair.c   |   12 +++------
 gfs2/fsck/util.c       |   21 ++++-----------
 gfs2/libgfs2/gfs2l.c   |    1 +
 gfs2/libgfs2/rgrp.c    |    4 +--
 9 files changed, 29 insertions(+), 110 deletions(-)

diff --git a/gfs2/edit/savemeta.c b/gfs2/edit/savemeta.c
index c38c5ac..676bc29 100644
--- a/gfs2/edit/savemeta.c
+++ b/gfs2/edit/savemeta.c
@@ -397,12 +397,12 @@ static int save_block(int fd, struct metafd *mfd, uint64_t blk)
  */
 static void save_ea_block(struct metafd *mfd, struct gfs2_buffer_head *metabh)
 {
-	int i, e, ea_len = sbd.bsize;
+	int e;
 	struct gfs2_ea_header ea;
 
-	for (e = sizeof(struct gfs2_meta_header); e < sbd.bsize; e += ea_len) {
+	for (e = sizeof(struct gfs2_meta_header); e < sbd.bsize; e += ea.ea_rec_len) {
 		uint64_t blk, *b;
-		int charoff;
+		int charoff, i;
 
 		gfs2_ea_header_in(&ea, metabh->b_data + e);
 		for (i = 0; i < ea.ea_num_ptrs; i++) {
@@ -417,7 +417,6 @@ static void save_ea_block(struct metafd *mfd, struct gfs2_buffer_head *metabh)
 		}
 		if (!ea.ea_rec_len)
 			break;
-		ea_len = ea.ea_rec_len;
 	}
 }
 
diff --git a/gfs2/fsck/inode_hash.c b/gfs2/fsck/inode_hash.c
index d5a35ce..aca5e61 100644
--- a/gfs2/fsck/inode_hash.c
+++ b/gfs2/fsck/inode_hash.c
@@ -46,15 +46,11 @@ struct inode_info *inodetree_insert(struct gfs2_inum di_num)
 			return cur;
 	}
 
-	data = malloc(sizeof(struct inode_info));
+	data = calloc(1, sizeof(struct inode_info));
 	if (!data) {
 		log_crit( _("Unable to allocate inode_info structure\n"));
 		return NULL;
 	}
-	if (!memset(data, 0, sizeof(struct inode_info))) {
-		log_crit( _("Error while zeroing inode_info structure\n"));
-		return NULL;
-	}
 	/* Add new node and rebalance tree. */
 	data->di_num.no_addr = di_num.no_addr;
 	data->di_num.no_formal_ino = di_num.no_formal_ino;
diff --git a/gfs2/fsck/main.c b/gfs2/fsck/main.c
index b4b1a03..346e580 100644
--- a/gfs2/fsck/main.c
+++ b/gfs2/fsck/main.c
@@ -154,7 +154,7 @@ static int check_statfs(struct gfs2_sbd *sdp)
 	struct osi_node *n, *next = NULL;
 	struct rgrp_tree *rgd;
 	struct gfs2_rindex *ri;
-	struct gfs2_statfs_change sc;
+	struct gfs2_statfs_change sc = {0,};
 	char buf[sizeof(struct gfs2_statfs_change)];
 	int count;
 
diff --git a/gfs2/fsck/pass2.c b/gfs2/fsck/pass2.c
index 9fd5be2..c8c047e 100644
--- a/gfs2/fsck/pass2.c
+++ b/gfs2/fsck/pass2.c
@@ -1413,7 +1413,6 @@ static int check_hash_tbl(struct gfs2_inode *ip, uint64_t *tbl,
 	struct gfs2_buffer_head *lbh;
 	int factor;
 	uint32_t proper_start;
-	uint32_t next_proper_start;
 	int anomaly;
 
 	lindex = 0;
@@ -1423,9 +1422,9 @@ static int check_hash_tbl(struct gfs2_inode *ip, uint64_t *tbl,
 		len = 1;
 		factor = 0;
 		leafblk = be64_to_cpu(tbl[lindex]);
-		next_proper_start = lindex;
 		anomaly = 0;
 		while (lindex + (len << 1) - 1 < hsize) {
+			uint32_t next_proper_start;
 			if (be64_to_cpu(tbl[lindex + (len << 1) - 1]) !=
 			    leafblk)
 				break;
@@ -1714,9 +1713,6 @@ static int check_system_dir(struct gfs2_inode *sysinode, const char *dirname,
 {
 	uint64_t iblock = 0, cur_blks;
 	struct dir_status ds = {0};
-	char *filename;
-	int filename_len;
-	char tmp_name[256];
 	int error = 0;
 
 	log_info( _("Checking system directory inode '%s'\n"), dirname);
@@ -1761,30 +1757,12 @@ static int check_system_dir(struct gfs2_inode *sysinode, const char *dirname,
 		log_err( _("No '.' entry found for %s directory.\n"), dirname);
 		if (query( _("Is it okay to add '.' entry? (y/n) "))) {
 			cur_blks = sysinode->i_di.di_blocks;
-			sprintf(tmp_name, ".");
-			filename_len = strlen(tmp_name); /* no trailing NULL */
-			if (!(filename = malloc(sizeof(char) * filename_len))) {
-				log_err( _("Unable to allocate name string\n"));
-				stack;
-				return -1;
-			}
-			if (!(memset(filename, 0, sizeof(char) *
-				    filename_len))) {
-				log_err( _("Unable to zero name string\n"));
-				stack;
-				free(filename);
-				return -1;
-			}
-			memcpy(filename, tmp_name, filename_len);
 			log_warn( _("Adding '.' entry\n"));
-			error = dir_add(sysinode, filename, filename_len,
-					&(sysinode->i_di.di_num),
-					(sysinode->i_sbd->gfs1 ?
-					 GFS_FILE_DIR : DT_DIR));
+			error = dir_add(sysinode, ".", 1, &(sysinode->i_di.di_num),
+			                (sysinode->i_sbd->gfs1 ? GFS_FILE_DIR : DT_DIR));
 			if (error) {
-				log_err(_("Error adding directory %s: %s\n"),
-				        filename, strerror(errno));
-				free(filename);
+				log_err(_("Error adding directory %s: %s\n"), "'.'",
+				         strerror(errno));
 				return -errno;
 			}
 			if (cur_blks != sysinode->i_di.di_blocks)
@@ -1793,7 +1771,6 @@ static int check_system_dir(struct gfs2_inode *sysinode, const char *dirname,
 			incr_link_count(sysinode->i_di.di_num, sysinode,
 					"sysinode \".\"");
 			ds.entry_count++;
-			free(filename);
 		} else
 			log_err( _("The directory was not fixed.\n"));
 	}
@@ -1874,9 +1851,6 @@ int pass2(struct gfs2_sbd *sdp)
 	uint8_t q;
 	struct dir_status ds = {0};
 	struct gfs2_inode *ip;
-	char *filename;
-	int filename_len;
-	char tmp_name[256];
 	int error = 0;
 
 	/* Check all the system directory inodes. */
@@ -2007,31 +1981,12 @@ int pass2(struct gfs2_sbd *sdp)
 				(unsigned long long)dirblk);
 
 			if (query( _("Is it okay to add '.' entry? (y/n) "))) {
-				sprintf(tmp_name, ".");
-				filename_len = strlen(tmp_name); /* no trailing
-								    NULL */
-				if (!(filename = malloc(sizeof(char) *
-						       filename_len))) {
-					log_err(_("Unable to allocate name\n"));
-					stack;
-					return FSCK_ERROR;
-				}
-				if (!memset(filename, 0, sizeof(char) *
-					   filename_len)) {
-					log_err( _("Unable to zero name\n"));
-					stack;
-					return FSCK_ERROR;
-				}
-				memcpy(filename, tmp_name, filename_len);
-
 				cur_blks = ip->i_di.di_blocks;
-				error = dir_add(ip, filename, filename_len,
-						&(ip->i_di.di_num),
-						(sdp->gfs1 ? GFS_FILE_DIR :
-						 DT_DIR));
+				error = dir_add(ip, ".", 1, &(ip->i_di.di_num),
+						(sdp->gfs1 ? GFS_FILE_DIR : DT_DIR));
 				if (error) {
-					log_err(_("Error adding directory %s: %s\n"),
-					        filename, strerror(errno));
+					log_err(_("Error adding directory %s: %s\n"), "'.'",
+					        strerror(errno));
 					return -errno;
 				}
 				if (cur_blks != ip->i_di.di_blocks) {
@@ -2047,7 +2002,6 @@ int pass2(struct gfs2_sbd *sdp)
 				incr_link_count(ip->i_di.di_num, ip,
 						_("\". (itself)\""));
 				ds.entry_count++;
-				free(filename);
 				log_err( _("The directory was fixed.\n"));
 			} else {
 				log_err( _("The directory was not fixed.\n"));
diff --git a/gfs2/fsck/pass3.c b/gfs2/fsck/pass3.c
index 4894d8c..6448da3 100644
--- a/gfs2/fsck/pass3.c
+++ b/gfs2/fsck/pass3.c
@@ -19,8 +19,9 @@
 static int attach_dotdot_to(struct gfs2_sbd *sdp, uint64_t newdotdot,
 			    uint64_t olddotdot, uint64_t block)
 {
-	char *filename;
-	int filename_len, err;
+	const char *filename = "..";
+	int filename_len = 2;
+	int err;
 	struct gfs2_inode *ip, *pip;
 	uint64_t cur_blks;
 
@@ -33,22 +34,6 @@ static int attach_dotdot_to(struct gfs2_sbd *sdp, uint64_t newdotdot,
 	 * '..' entry for this directory in
 	 * this case? */
 
-	filename_len = strlen("..");
-	if (!(filename = malloc((sizeof(char) * filename_len) + 1))) {
-		log_err( _("Unable to allocate name\n"));
-		fsck_inode_put(&ip);
-		fsck_inode_put(&pip);
-		stack;
-		return -1;
-	}
-	if (!memset(filename, 0, (sizeof(char) * filename_len) + 1)) {
-		log_err( _("Unable to zero name\n"));
-		fsck_inode_put(&ip);
-		fsck_inode_put(&pip);
-		stack;
-		return -1;
-	}
-	memcpy(filename, "..", filename_len);
 	if (gfs2_dirent_del(ip, filename, filename_len))
 		log_warn( _("Unable to remove \"..\" directory entry.\n"));
 	else
@@ -72,7 +57,6 @@ static int attach_dotdot_to(struct gfs2_sbd *sdp, uint64_t newdotdot,
 	incr_link_count(pip->i_di.di_num, ip, _("new \"..\""));
 	fsck_inode_put(&ip);
 	fsck_inode_put(&pip);
-	free(filename);
 	return 0;
 }
 
diff --git a/gfs2/fsck/rgrepair.c b/gfs2/fsck/rgrepair.c
index 2c1d613..4fdbcf7 100644
--- a/gfs2/fsck/rgrepair.c
+++ b/gfs2/fsck/rgrepair.c
@@ -327,7 +327,6 @@ static uint64_t find_next_rgrp_dist(struct gfs2_sbd *sdp, uint64_t blk,
 			rgrp_dist++;
 		}
 		if (found) {
-			block = next_block;
 			log_info( _("rgrp found at 0x%llx, length=%d, "
 				    "used=%llu, free=%d\n"),
 				  prevrgd->ri.ri_addr, length,
@@ -442,7 +441,7 @@ static int gfs2_rindex_rebuild(struct gfs2_sbd *sdp, int *num_rgs,
 	uint64_t first_rg_dist, initial_first_rg_dist;
 	struct rgrp_tree *calc_rgd, *prev_rgd;
 	int number_of_rgs, rgi;
-	int rg_was_fnd = FALSE, corrupt_rgs = 0, bitmap_was_fnd;
+	int rg_was_fnd = FALSE, corrupt_rgs = 0;
 
 	sdp->rgcalc.osi_node = NULL;
 	initial_first_rg_dist = first_rg_dist = sdp->sb_addr + 1;
@@ -489,13 +488,10 @@ static int gfs2_rindex_rebuild(struct gfs2_sbd *sdp, int *num_rgs,
 		/* ------------------------------------------------ */
 		/* Now go through and count the bitmaps for this RG */
 		/* ------------------------------------------------ */
-		bitmap_was_fnd = FALSE;
-		for (fwd_block = blk + 1;
-		     fwd_block < sdp->device.length; 
-		     fwd_block++) {
+		for (fwd_block = blk + 1; fwd_block < sdp->device.length; fwd_block++) {
+			int bitmap_was_fnd;
 			bh = bread(sdp, fwd_block);
-			bitmap_was_fnd =
-				(!gfs2_check_meta(bh, GFS2_METATYPE_RB));
+			bitmap_was_fnd = !gfs2_check_meta(bh, GFS2_METATYPE_RB);
 			brelse(bh);
 			if (bitmap_was_fnd) /* if a bitmap */
 				calc_rgd->ri.ri_length++;
diff --git a/gfs2/fsck/util.c b/gfs2/fsck/util.c
index 3e3050f..f8ccb3e 100644
--- a/gfs2/fsck/util.c
+++ b/gfs2/fsck/util.c
@@ -365,14 +365,9 @@ int add_duplicate_ref(struct gfs2_inode *ip, uint64_t block,
 		/* Check for the inode on the invalid inode reference list. */
 		uint8_t q;
 
-		if (!(id = malloc(sizeof(*id)))) {
-			log_crit( _("Unable to allocate "
-				    "inode_with_dups structure\n"));
-			return meta_error;
-		}
-		if (!(memset(id, 0, sizeof(*id)))) {
-			log_crit( _("Unable to zero inode_with_dups "
-				    "structure\n"));
+		id = calloc(1, sizeof(*id));
+		if (!id) {
+			log_crit( _("Unable to allocate inode_with_dups structure\n"));
 			return meta_error;
 		}
 		id->block_no = ip->i_di.di_num.no_addr;
@@ -429,15 +424,11 @@ struct dir_info *dirtree_insert(struct gfs2_inum inum)
 			return cur;
 	}
 
-	data = malloc(sizeof(struct dir_info));
+	data = calloc(1, sizeof(struct dir_info));
 	if (!data) {
 		log_crit( _("Unable to allocate dir_info structure\n"));
 		return NULL;
 	}
-	if (!memset(data, 0, sizeof(struct dir_info))) {
-		log_crit( _("Error while zeroing dir_info structure\n"));
-		return NULL;
-	}
 	/* Add new node and rebalance tree. */
 	data->dinode.no_addr = inum.no_addr;
 	data->dinode.no_formal_ino = inum.no_formal_ino;
@@ -555,8 +546,8 @@ struct gfs2_bmap *gfs2_bmap_create(struct gfs2_sbd *sdp, uint64_t size,
 	struct gfs2_bmap *il;
 
 	*addl_mem_needed = 0L;
-	il = malloc(sizeof(*il));
-	if (!il || !memset(il, 0, sizeof(*il)))
+	il = calloc(1, sizeof(*il));
+	if (!il)
 		return NULL;
 
 	if (gfs2_blockmap_create(il, size)) {
diff --git a/gfs2/libgfs2/gfs2l.c b/gfs2/libgfs2/gfs2l.c
index e58c827..eda9531 100644
--- a/gfs2/libgfs2/gfs2l.c
+++ b/gfs2/libgfs2/gfs2l.c
@@ -172,6 +172,7 @@ int main(int argc, char *argv[])
 	ret = lgfs2_lang_parsef(state, opts.src);
 	if (ret != 0) {
 		fprintf(stderr, "Parse failed\n");
+		free(opts.fspath);
 		return ret;
 	}
 
diff --git a/gfs2/libgfs2/rgrp.c b/gfs2/libgfs2/rgrp.c
index 8fe7b6d..0752772 100644
--- a/gfs2/libgfs2/rgrp.c
+++ b/gfs2/libgfs2/rgrp.c
@@ -182,11 +182,9 @@ struct rgrp_tree *rgrp_insert(struct osi_root *rgtree, uint64_t rgblock)
 			return cur;
 	}
 
-	data = malloc(sizeof(struct rgrp_tree));
+	data = calloc(1, sizeof(struct rgrp_tree));
 	if (!data)
 		return NULL;
-	if (!memset(data, 0, sizeof(struct rgrp_tree)))
-		return NULL;
 	/* Add new node and rebalance tree. */
 	data->ri.ri_addr = rgblock;
 	osi_link_node(&data->node, parent, newn);


More information about the cluster-commits mailing list