diff mbox

[5/7] misc: fix Coverity errors

Message ID 147200549633.15538.18051281375686885659.stgit@birch.djwong.org (mailing list archive)
State Accepted
Headers show

Commit Message

Darrick J. Wong Aug. 24, 2016, 2:24 a.m. UTC 
Fix various code sloppinesses pointed out by Coverity.

Coverity-id: 1371628 - 1371638
Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 db/fsmap.c      |    1 +
 repair/phase5.c |   12 ++++++++----
 repair/rmap.c   |    5 +++--
 repair/scan.c   |    1 +
 4 files changed, 13 insertions(+), 6 deletions(-)

Comments

Christoph Hellwig Aug. 25, 2016, 9:14 a.m. UTC | #1 
On Tue, Aug 23, 2016 at 07:24:56PM -0700, Darrick J. Wong wrote:
> Fix various code sloppinesses pointed out by Coverity.
> 
> Coverity-id: 1371628 - 1371638

> @@ -75,6 +75,7 @@ fsmap(
>  	high.rm_owner = ULLONG_MAX;
>  	high.rm_offset = ULLONG_MAX;
>  	high.rm_flags = XFS_RMAP_ATTR_FORK | XFS_RMAP_BMBT_BLOCK | XFS_RMAP_UNWRITTEN;
> +	high.rm_blockcount = low.rm_blockcount = 0;

Do the low initialization near the remaining low fields?

or better do a struct initialization ala

	struct xfs_rmap_irec low = { 0, };
	struct xfs_rmap_irec high = { 0, };

that ensures the who;le structure is zero-filled for uninitialized
fields.

> diff --git a/repair/phase5.c b/repair/phase5.c
> index e583879..5a7185c 100644
> --- a/repair/phase5.c
> +++ b/repair/phase5.c
> @@ -1464,7 +1464,7 @@ prop_rmap_cursor(
>  		 * and set the rightsib pointer of current block
>  		 */
>  #ifdef XR_BLD_INO_TRACE
> -		fprintf(stderr, " ino prop agbno %d ", lptr->prev_agbno);
> +		fprintf(stderr, " rmap prop agbno %d ", lptr->prev_agbno);
>  #endif

Did Coveryity really point this out? :)

> @@ -1548,6 +1548,7 @@ prop_rmap_highkey(
>  		bt_key->rm_offset = cpu_to_be64(
>  				libxfs_rmap_irec_offset_pack(&high_key));
>  
> +		key.rm_blockcount = 0;

should probably be a struct initializer again
Darrick J. Wong Aug. 25, 2016, 4:30 p.m. UTC | #2 
On Thu, Aug 25, 2016 at 02:14:52AM -0700, Christoph Hellwig wrote:
> On Tue, Aug 23, 2016 at 07:24:56PM -0700, Darrick J. Wong wrote:
> > Fix various code sloppinesses pointed out by Coverity.
> > 
> > Coverity-id: 1371628 - 1371638
> 
> > @@ -75,6 +75,7 @@ fsmap(
> >  	high.rm_owner = ULLONG_MAX;
> >  	high.rm_offset = ULLONG_MAX;
> >  	high.rm_flags = XFS_RMAP_ATTR_FORK | XFS_RMAP_BMBT_BLOCK | XFS_RMAP_UNWRITTEN;
> > +	high.rm_blockcount = low.rm_blockcount = 0;
> 
> Do the low initialization near the remaining low fields?
> 
> or better do a struct initialization ala
> 
> 	struct xfs_rmap_irec low = { 0, };
> 	struct xfs_rmap_irec high = { 0, };
> 
> that ensures the who;le structure is zero-filled for uninitialized
> fields.

Ok.

> > diff --git a/repair/phase5.c b/repair/phase5.c
> > index e583879..5a7185c 100644
> > --- a/repair/phase5.c
> > +++ b/repair/phase5.c
> > @@ -1464,7 +1464,7 @@ prop_rmap_cursor(
> >  		 * and set the rightsib pointer of current block
> >  		 */
> >  #ifdef XR_BLD_INO_TRACE
> > -		fprintf(stderr, " ino prop agbno %d ", lptr->prev_agbno);
> > +		fprintf(stderr, " rmap prop agbno %d ", lptr->prev_agbno);
> >  #endif
> 
> Did Coveryity really point this out? :)

Heh, nope.  I'll add it in the changelog.

> > @@ -1548,6 +1548,7 @@ prop_rmap_highkey(
> >  		bt_key->rm_offset = cpu_to_be64(
> >  				libxfs_rmap_irec_offset_pack(&high_key));
> >  
> > +		key.rm_blockcount = 0;
> 
> should probably be a struct initializer again

<nod>

--D
diff mbox

Patch

diff --git a/db/fsmap.c b/db/fsmap.c
index b2ba55d..4b245b9 100644
--- a/db/fsmap.c
+++ b/db/fsmap.c
@@ -75,6 +75,7 @@  fsmap(
 	high.rm_owner = ULLONG_MAX;
 	high.rm_offset = ULLONG_MAX;
 	high.rm_flags = XFS_RMAP_ATTR_FORK | XFS_RMAP_BMBT_BLOCK | XFS_RMAP_UNWRITTEN;
+	high.rm_blockcount = low.rm_blockcount = 0;
 
 	start_ag = XFS_FSB_TO_AGNO(mp, start_fsb);
 	end_ag = XFS_FSB_TO_AGNO(mp, end_fsb);
diff --git a/repair/phase5.c b/repair/phase5.c
index e583879..5a7185c 100644
--- a/repair/phase5.c
+++ b/repair/phase5.c
@@ -1464,7 +1464,7 @@  prop_rmap_cursor(
 		 * and set the rightsib pointer of current block
 		 */
 #ifdef XR_BLD_INO_TRACE
-		fprintf(stderr, " ino prop agbno %d ", lptr->prev_agbno);
+		fprintf(stderr, " rmap prop agbno %d ", lptr->prev_agbno);
 #endif
 		if (lptr->prev_agbno != NULLAGBLOCK)  {
 			ASSERT(lptr->prev_buf_p != NULL);
@@ -1502,7 +1502,7 @@  prop_rmap_cursor(
 		prop_rmap_cursor(mp, agno, btree_curs, rm_rec, level);
 	}
 	/*
-	 * add inode info to current block
+	 * add rmap info to current block
 	 */
 	be16_add_cpu(&bt_hdr->bb_numrecs, 1);
 
@@ -1548,6 +1548,7 @@  prop_rmap_highkey(
 		bt_key->rm_offset = cpu_to_be64(
 				libxfs_rmap_irec_offset_pack(&high_key));
 
+		key.rm_blockcount = 0;
 		for (i = 1; i < numrecs - 1; i++) {
 			bt_key = XFS_RMAP_HIGH_KEY_ADDR(bt_hdr, i);
 			key.rm_startblock = be32_to_cpu(bt_key->rm_startblock);
@@ -1621,7 +1622,7 @@  _("Insufficient memory to construct reverse-map cursor."));
 	rm_rec = pop_slab_cursor(rmap_cur);
 	lptr = &btree_curs->level[0];
 
-	for (i = 0; i < lptr->num_blocks; i++)  {
+	for (i = 0; i < lptr->num_blocks && rm_rec != NULL; i++)  {
 		/*
 		 * block initialization, lay in block header
 		 */
@@ -1639,14 +1640,17 @@  _("Insufficient memory to construct reverse-map cursor."));
 		if (lptr->modulo > 0)
 			lptr->modulo--;
 
-		if (lptr->num_recs_pb > 0)
+		if (lptr->num_recs_pb > 0) {
+			ASSERT(rm_rec != NULL);
 			prop_rmap_cursor(mp, agno, btree_curs, rm_rec, 0);
+		}
 
 		bt_rec = (struct xfs_rmap_rec *)
 			  ((char *)bt_hdr + XFS_RMAP_BLOCK_LEN);
 		highest_key.rm_startblock = 0;
 		highest_key.rm_owner = 0;
 		highest_key.rm_offset = 0;
+		highest_key.rm_blockcount = 0;
 		for (j = 0; j < be16_to_cpu(bt_hdr->bb_numrecs); j++) {
 			ASSERT(rm_rec != NULL);
 			bt_rec[j].rm_startblock =
diff --git a/repair/rmap.c b/repair/rmap.c
index f22f4f0..b3d4c25 100644
--- a/repair/rmap.c
+++ b/repair/rmap.c
@@ -316,7 +316,7 @@  fold_raw_rmaps(
 	struct xfs_slab_cursor	*cur = NULL;
 	struct xfs_rmap_irec	*prev, *rec;
 	size_t			old_sz;
-	int			error;
+	int			error = 0;
 
 	old_sz = slab_count(ag_rmaps[agno].ar_rmaps);
 	if (slab_count(ag_rmaps[agno].ar_raw_rmaps) == 0)
@@ -329,7 +329,7 @@  fold_raw_rmaps(
 
 	prev = pop_slab_cursor(cur);
 	rec = pop_slab_cursor(cur);
-	while (rec) {
+	while (prev && rec) {
 		if (mergeable_rmaps(prev, rec)) {
 			prev->rm_blockcount += rec->rm_blockcount;
 			rec = pop_slab_cursor(cur);
@@ -843,6 +843,7 @@  rmap_high_key_from_rec(
 	    (rec->rm_flags & XFS_RMAP_BMBT_BLOCK))
 		return;
 	key->rm_offset += adj;
+	key->rm_blockcount = 0;
 }
 
 /*
diff --git a/repair/scan.c b/repair/scan.c
index 9a46dd0..253e3de 100644
--- a/repair/scan.c
+++ b/repair/scan.c
@@ -1109,6 +1109,7 @@  advance:
 		key.rm_flags = 0;
 		key.rm_startblock = be32_to_cpu(kp->rm_startblock);
 		key.rm_owner = be64_to_cpu(kp->rm_owner);
+		key.rm_blockcount = 0;
 		if (libxfs_rmap_irec_offset_unpack(be64_to_cpu(kp->rm_offset),
 				&key)) {
 			/* Look for impossible flags. */