XFS: Fix gcc 4.6 set but not read and unused statement warnings v2

PATCH In review Andi Kleen 5 participants 10 messages Jun 14, 2010 – Jun 16, 2010 XFS filesystem

lore

Share & tools

lore

Andi Kleen

author

2010-06-14 08:11 UTC #0 lore raw

PATCH 1/1 XFS: Fix gcc 4.6 set but not read and unused statement warnings v2

XFS: Fix gcc 4.6 set but not read and unused statement warnings v2

Addresses (near) all outstanding comments:

v2: QASSERT removed. Dave Chinners comments addressed (well the one I liked). CONFIG_XFS_DEBUG builds again now. I checked RT+QUOTA enabled and there was no new warnings (if someone is interested i put the remaining ones at http://halobates.de/xfs-all-gcc46)

-Andi

---

For my configuration, that is without quota or RT.

Mostly dead code removed I think (but needs additional review)

That is there were one or two bad error handling cases, but they were not easily fixable, with comments and I left the warnings in for those for you to remember.

e.g. if there is a ENOSPC down in xfs_trans.c while modifying the superblock it would not be handled.

Unused statements were mostly related to stub macros for disabled features like QUOTA or RT ALLOC. I replace those with inlines.

There were also some problems with variables used in ASSERT() I partly moved those into the ASSERT itself.

Cc [email protected]

Signed-off-by Andi Kleen <[email protected]>

---
 fs/xfs/linux-2.6/xfs_sync.c |    3 +++
 fs/xfs/xfs_alloc.c          |   10 +++-------
 fs/xfs/xfs_da_btree.c       |   19 +++++++------------
 fs/xfs/xfs_dir2_block.c     |    6 +++---
 fs/xfs/xfs_filestream.c     |   16 +++++-----------
 fs/xfs/xfs_iget.c           |    3 ---
 fs/xfs/xfs_inode.c          |    4 ----
 fs/xfs/xfs_inode_item.c     |    8 ++------
 fs/xfs/xfs_log.c            |    2 --
 fs/xfs/xfs_quota.h          |   14 ++++++++++----
 fs/xfs/xfs_trans.c          |    6 +++++-
 11 files changed, 38 insertions(+), 53 deletions(-)

fs/xfs/linux-2.6/xfs_sync.c +3 -0

Index: linux-2.6.35-rc3-gcc/fs/xfs/linux-2.6/xfs_sync.c
===================================================================
@@ -554,6 +554,9 @@ xfs_sync_worker(
 		xfs_log_force(mp, 0);
 		xfs_reclaim_inodes(mp, 0);
 		/* dgc: errors ignored here */
+		/* ak: yes and you'll get a warning for it now when you
+		 * upgrade compilers.
+		 */
 		error = xfs_qm_sync(mp, SYNC_TRYLOCK);
 		if (xfs_log_need_covered(mp))
 			error = xfs_commit_dummy_trans(mp, 0);

fs/xfs/xfs_da_btree.c +7 ~~-12~~

Index: linux-2.6.35-rc3-gcc/fs/xfs/xfs_da_btree.c
===================================================================
@@ -581,16 +581,14 @@ xfs_da_node_add(xfs_da_state_t *state, x
 	xfs_da_intnode_t *node;
 	xfs_da_node_entry_t *btree;
 	int tmp;
-	xfs_mount_t *mp;
 
 	node = oldblk->bp->data;
-	mp = state->mp;
 	ASSERT(be16_to_cpu(node->hdr.info.magic) == XFS_DA_NODE_MAGIC);
 	ASSERT((oldblk->index >= 0) && (oldblk->index <= be16_to_cpu(node->hdr.count)));
 	ASSERT(newblk->blkno != 0);
 	if (state->args->whichfork == XFS_DATA_FORK)
-		ASSERT(newblk->blkno >= mp->m_dirleafblk &&
-		       newblk->blkno < mp->m_dirfreeblk);
+		ASSERT(newblk->blkno >= state->mp->m_dirleafblk &&
+		       newblk->blkno < state->mp->m_dirfreeblk);
 
 	/*
 	 * We may need to make some room before we insert the new node.

@@ -710,8 +708,6 @@ STATIC int
 xfs_da_root_join(xfs_da_state_t *state, xfs_da_state_blk_t *root_blk)
 {
 	xfs_da_intnode_t *oldroot;
-	/* REFERENCED */
-	xfs_da_blkinfo_t *blkinfo;
 	xfs_da_args_t *args;
 	xfs_dablk_t child;
 	xfs_dabuf_t *bp;

@@ -742,15 +738,14 @@ xfs_da_root_join(xfs_da_state_t *state,
 	if (error)
 		return(error);
 	ASSERT(bp != NULL);
-	blkinfo = bp->data;
 	if (be16_to_cpu(oldroot->hdr.level) == 1) {
-		ASSERT(be16_to_cpu(blkinfo->magic) == XFS_DIR2_LEAFN_MAGIC ||
-		       be16_to_cpu(blkinfo->magic) == XFS_ATTR_LEAF_MAGIC);
+		ASSERT(be16_to_cpu(bp->data->magic) == XFS_DIR2_LEAFN_MAGIC ||
+		       be16_to_cpu(bp->data->magic) == XFS_ATTR_LEAF_MAGIC);
 	} else {
-		ASSERT(be16_to_cpu(blkinfo->magic) == XFS_DA_NODE_MAGIC);
+		ASSERT(be16_to_cpu(bp->data->magic) == XFS_DA_NODE_MAGIC);
 	}
-	ASSERT(!blkinfo->forw);
-	ASSERT(!blkinfo->back);
+	ASSERT(!bp->data->forw);
+	ASSERT(!bp->data->back);
 	memcpy(root_blk->bp->data, bp->data, state->blocksize);
 	xfs_da_log_buf(args->trans, root_blk->bp, 0, state->blocksize - 1);
 	error = xfs_da_shrink_inode(args, child, bp);

fs/xfs/xfs_dir2_block.c +3 -3

Index: linux-2.6.35-rc3-gcc/fs/xfs/xfs_dir2_block.c
===================================================================
@@ -1073,10 +1073,10 @@ xfs_dir2_sf_to_block(
 	 */
 
 	buf_len = dp->i_df.if_bytes;
-	buf = kmem_alloc(dp->i_df.if_bytes, KM_SLEEP);
+	buf = kmem_alloc(buf_len, KM_SLEEP);
 
-	memcpy(buf, sfp, dp->i_df.if_bytes);
-	xfs_idata_realloc(dp, -dp->i_df.if_bytes, XFS_DATA_FORK);
+	memcpy(buf, sfp, buf_len);
+	xfs_idata_realloc(dp, -buf_len, XFS_DATA_FORK);
 	dp->i_d.di_size = 0;
 	xfs_trans_log_inode(tp, dp, XFS_ILOG_CORE);
 	/*

fs/xfs/xfs_filestream.c +5 ~~-11~~

Index: linux-2.6.35-rc3-gcc/fs/xfs/xfs_filestream.c
===================================================================
@@ -140,9 +140,8 @@ _xfs_filestream_pick_ag(
 	int		flags,
 	xfs_extlen_t	minlen)
 {
-	int		streams, max_streams;
 	int		err, trylock, nscan;
-	xfs_extlen_t	longest, free, minfree, maxfree = 0;
+	xfs_extlen_t	longest, minfree, maxfree = 0;
 	xfs_agnumber_t	ag, max_ag = NULLAGNUMBER;
 	struct xfs_perag *pag;

@@ -174,7 +173,6 @@ _xfs_filestream_pick_ag(
 		/* Keep track of the AG with the most free blocks. */
 		if (pag->pagf_freeblks > maxfree) {
 			maxfree = pag->pagf_freeblks;
-			max_streams = atomic_read(&pag->pagf_fstrms);
 			max_ag = ag;
 		}

@@ -196,8 +194,6 @@ _xfs_filestream_pick_ag(
 		     (flags & XFS_PICK_LOWSPACE))) {
 
 			/* Break out, retaining the reference on the AG. */
-			free = pag->pagf_freeblks;
-			streams = atomic_read(&pag->pagf_fstrms);
 			xfs_perag_put(pag);
 			*agp = ag;
 			break;

@@ -234,8 +230,6 @@ next_ag:
 		if (max_ag != NULLAGNUMBER) {
 			xfs_filestream_get_ag(mp, max_ag);
 			TRACE_AG_PICK1(mp, max_ag, maxfree);
-			streams = max_streams;
-			free = maxfree;
 			*agp = max_ag;
 			break;
 		}

@@ -364,7 +358,8 @@ xfs_fstrm_free_func(
 	/* Drop the reference taken on the AG when the item was added. */
 	ref = xfs_filestream_put_ag(ip->i_mount, item->ag);
 
-	ASSERT(ref >= 0);
+	if (ref < 0)
+		ASSERT(0);
 	TRACE_FREE(ip->i_mount, ip, item->pip, item->ag,
 		xfs_filestream_peek_ag(ip->i_mount, item->ag));

@@ -465,7 +460,6 @@ xfs_filestream_lookup_ag(
 	xfs_mru_cache_t	*cache;
 	fstrm_item_t	*item;
 	xfs_agnumber_t	ag;
-	int		ref;
 
 	if (!(ip->i_d.di_mode & (S_IFREG | S_IFDIR))) {
 		ASSERT(0);

@@ -481,10 +475,10 @@ xfs_filestream_lookup_ag(
 
 	ASSERT(ip == item->ip);
 	ag = item->ag;
-	ref = xfs_filestream_peek_ag(ip->i_mount, ag);
 	xfs_mru_cache_done(cache);
 
-	TRACE_LOOKUP(ip->i_mount, ip, item->pip, ag, ref);
+	TRACE_LOOKUP(ip->i_mount, ip, item->pip, ag,
+			xfs_filestream_peek_ag(ip->i_mount, ag));
 	return ag;
 }

fs/xfs/xfs_iget.c +0 -3

Index: linux-2.6.35-rc3-gcc/fs/xfs/xfs_iget.c
===================================================================
@@ -265,7 +265,6 @@ xfs_iget_cache_miss(
 {
 	struct xfs_inode	*ip;
 	int			error;
-	unsigned long		first_index, mask;
 	xfs_agino_t		agino = XFS_INO_TO_AGINO(mp, ino);
 
 	ip = xfs_inode_alloc(mp, ino);

@@ -302,8 +301,6 @@ xfs_iget_cache_miss(
 			BUG();
 	}
 
-	mask = ~(((XFS_INODE_CLUSTER_SIZE(mp) >> mp->m_sb.sb_inodelog)) - 1);
-	first_index = agino & mask;
 	write_lock(&pag->pag_ici_lock);
 
 	/* insert the new inode */

fs/xfs/xfs_inode.c +0 -4

Index: linux-2.6.35-rc3-gcc/fs/xfs/xfs_inode.c
===================================================================
@@ -925,7 +925,6 @@ xfs_iread_extents(
 	int		error;
 	xfs_ifork_t	*ifp;
 	xfs_extnum_t	nextents;
-	size_t		size;
 
 	if (unlikely(XFS_IFORK_FORMAT(ip, whichfork) != XFS_DINODE_FMT_BTREE)) {
 		XFS_ERROR_REPORT("xfs_iread_extents", XFS_ERRLEVEL_LOW,

@@ -933,7 +932,6 @@ xfs_iread_extents(
 		return XFS_ERROR(EFSCORRUPTED);
 	}
 	nextents = XFS_IFORK_NEXTENTS(ip, whichfork);
-	size = nextents * sizeof(xfs_bmbt_rec_t);
 	ifp = XFS_IFORK_PTR(ip, whichfork);
 
 	/*

@@ -3517,13 +3515,11 @@ xfs_iext_remove_indirect(
 	xfs_extnum_t	ext_diff;	/* extents to remove in current list */
 	xfs_extnum_t	nex1;		/* number of extents before idx */
 	xfs_extnum_t	nex2;		/* extents after idx + count */
-	int		nlists;		/* entries in indirection array */
 	int		page_idx = idx;	/* index in target extent list */
 
 	ASSERT(ifp->if_flags & XFS_IFEXTIREC);
 	erp = xfs_iext_idx_to_irec(ifp,  &page_idx, &erp_idx, 0);
 	ASSERT(erp != NULL);
-	nlists = ifp->if_real_bytes / XFS_IEXT_BUFSZ;
 	nex1 = page_idx;
 	ext_cnt = count;
 	while (ext_cnt) {

fs/xfs/xfs_inode_item.c +2 -6

Index: linux-2.6.35-rc3-gcc/fs/xfs/xfs_inode_item.c
===================================================================
@@ -220,7 +220,6 @@ xfs_inode_item_format(
 	xfs_inode_t		*ip;
 	size_t			data_bytes;
 	xfs_bmbt_rec_t		*ext_buffer;
-	int			nrecs;
 	xfs_mount_t		*mp;
 
 	ip = iip->ili_inode;

@@ -323,9 +322,8 @@ xfs_inode_item_format(
 			ASSERT(ip->i_df.if_u1.if_extents != NULL);
 			ASSERT(ip->i_d.di_nextents > 0);
 			ASSERT(iip->ili_extents_buf == NULL);
-			nrecs = ip->i_df.if_bytes /
-				(uint)sizeof(xfs_bmbt_rec_t);
-			ASSERT(nrecs > 0);
+			ASSERT((ip->i_df.if_bytes /
+				(uint)sizeof(xfs_bmbt_rec_t)) > 0);
 #ifdef XFS_NATIVE_HOST
 			if (nrecs == ip->i_d.di_nextents) {
 				/*

@@ -957,10 +955,8 @@ xfs_iflush_abort(
 	xfs_inode_t		*ip)
 {
 	xfs_inode_log_item_t	*iip = ip->i_itemp;
-	xfs_mount_t		*mp;
 
 	iip = ip->i_itemp;
-	mp = ip->i_mount;
 	if (iip) {
 		struct xfs_ail	*ailp = iip->ili_item.li_ailp;
 		if (iip->ili_item.li_flags & XFS_LI_IN_AIL) {

fs/xfs/xfs_log.c +0 -2

Index: linux-2.6.35-rc3-gcc/fs/xfs/xfs_log.c
===================================================================
@@ -1047,7 +1047,6 @@ xlog_alloc_log(xfs_mount_t	*mp,
 	xlog_in_core_t		*iclog, *prev_iclog=NULL;
 	xfs_buf_t		*bp;
 	int			i;
-	int			iclogsize;
 	int			error = ENOMEM;
 	uint			log2_size = 0;

@@ -1127,7 +1126,6 @@ xlog_alloc_log(xfs_mount_t	*mp,
 	 * with different amounts of memory.  See the definition of
 	 * xlog_in_core_t in xfs_log_priv.h for details.
 	 */
-	iclogsize = log->l_iclog_size;
 	ASSERT(log->l_iclog_size >= 4096);
 	for (i=0; i < log->l_iclog_bufs; i++) {
 		*iclogp = kmem_zalloc(sizeof(xlog_in_core_t), KM_MAYFAIL);

fs/xfs/xfs_quota.h +10 -4

Index: linux-2.6.35-rc3-gcc/fs/xfs/xfs_quota.h
===================================================================
@@ -346,7 +346,13 @@ xfs_qm_vop_dqalloc(struct xfs_inode *ip,
 #define xfs_trans_mod_dquot_byino(tp, ip, fields, delta)
 #define xfs_trans_apply_dquot_deltas(tp)
 #define xfs_trans_unreserve_and_mod_dquots(tp)
-#define xfs_trans_reserve_quota_nblks(tp, ip, nblks, ninos, flags)	(0)
+
+static inline int xfs_trans_reserve_quota_nblks(struct xfs_trans *t,
+		struct xfs_inode *i, long a, long b, uint c)
+{
+	return 0;
+}
+
 #define xfs_trans_reserve_quota_bydquots(tp, mp, u, g, nb, ni, fl)	(0)
 #define xfs_qm_vop_create_dqattach(tp, ip, u, g)
 #define xfs_qm_vop_rename_dqattach(it)					(0)

@@ -355,13 +361,13 @@ xfs_qm_vop_dqalloc(struct xfs_inode *ip,
 #define xfs_qm_dqattach(ip, fl)						(0)
 #define xfs_qm_dqattach_locked(ip, fl)					(0)
 #define xfs_qm_dqdetach(ip)
-#define xfs_qm_dqrele(d)
+static inline void xfs_qm_dqrele(struct xfs_dquot *d) {}
 #define xfs_qm_statvfs(ip, s)
-#define xfs_qm_sync(mp, fl)						(0)
+static inline int xfs_qm_sync(struct xfs_mount *m, int i) { return 0; }
 #define xfs_qm_newmount(mp, a, b)					(0)
 #define xfs_qm_mount_quotas(mp)
 #define xfs_qm_unmount(mp)
-#define xfs_qm_unmount_quotas(mp)					(0)
+static inline void xfs_qm_unmount_quotas(struct xfs_mount *m) {}
 #endif /* CONFIG_XFS_QUOTA */
 
 #define xfs_trans_unreserve_quota_nblks(tp, ip, nblks, ninos, flags) \

fs/xfs/xfs_trans.c +5 -1

Index: linux-2.6.35-rc3-gcc/fs/xfs/xfs_trans.c
===================================================================
@@ -1119,7 +1119,11 @@ xfs_trans_unreserve_and_mod_sb(
 	if (msbp > msb) {
 		error = xfs_mod_incore_sb_batch(tp->t_mountp, msb,
 			(uint)(msbp - msb), rsvd);
-		ASSERT(error == 0);
+		/*
+	 	 * When an error happens here it's a race that's supposed
+		 * to be fixed. Check for this unconditionally.
+		 */
+		BUG_ON(error != 0);
 	}
 }

fs/xfs/xfs_alloc.c +3 -7

Index: linux-2.6.35-rc3-gcc/fs/xfs/xfs_alloc.c
===================================================================
@@ -688,8 +688,6 @@ xfs_alloc_ag_vextent_near(
 	xfs_agblock_t	ltbno;		/* start bno of left side entry */
 	xfs_agblock_t	ltbnoa;		/* aligned ... */
 	xfs_extlen_t	ltdiff;		/* difference to left side entry */
-	/*REFERENCED*/
-	xfs_agblock_t	ltend;		/* end bno of left side entry */
 	xfs_extlen_t	ltlen;		/* length of left side entry */
 	xfs_extlen_t	ltlena;		/* aligned ... */
 	xfs_agblock_t	ltnew;		/* useful start bno of left side */

@@ -814,8 +812,7 @@ xfs_alloc_ag_vextent_near(
 		if ((error = xfs_alloc_get_rec(cnt_cur, &ltbno, &ltlen, &i)))
 			goto error0;
 		XFS_WANT_CORRUPTED_GOTO(i == 1, error0);
-		ltend = ltbno + ltlen;
-		ASSERT(ltend <= be32_to_cpu(XFS_BUF_TO_AGF(args->agbp)->agf_length));
+		ASSERT(ltbno + ltlen <= be32_to_cpu(XFS_BUF_TO_AGF(args->agbp)->agf_length));
 		args->len = blen;
 		if (!xfs_alloc_fix_minleft(args)) {
 			xfs_btree_del_cursor(cnt_cur, XFS_BTREE_NOERROR);

@@ -828,7 +825,7 @@ xfs_alloc_ag_vextent_near(
 		 */
 		args->agbno = bnew;
 		ASSERT(bnew >= ltbno);
-		ASSERT(bnew + blen <= ltend);
+		ASSERT(bnew + blen <= ltbno + ltlen);
 		/*
 		 * Set up a cursor for the by-bno tree.
 		 */

@@ -1157,7 +1154,6 @@ xfs_alloc_ag_vextent_near(
 	/*
 	 * Fix up the length and compute the useful address.
 	 */
-	ltend = ltbno + ltlen;
 	args->len = XFS_EXTLEN_MIN(ltlena, args->maxlen);
 	xfs_alloc_fix_len(args);
 	if (!xfs_alloc_fix_minleft(args)) {

@@ -1170,7 +1166,7 @@ xfs_alloc_ag_vextent_near(
 	(void)xfs_alloc_compute_diff(args->agbno, rlen, args->alignment, ltbno,
 		ltlen, &ltnew);
 	ASSERT(ltnew >= ltbno);
-	ASSERT(ltnew + rlen <= ltend);
+	ASSERT(ltnew + rlen <= ltbno + ltlen);
 	ASSERT(ltnew + rlen <= be32_to_cpu(XFS_BUF_TO_AGF(args->agbp)->agf_length));
 	args->agbno = ltnew;
 	if ((error = xfs_alloc_fixup_trees(cnt_cur, bno_cur_lt, ltbno, ltlen,

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs

Christoph Hellwig

2010-06-14 08:28 UTC #1 lore raw

Quoted 8 lines · Andi Kleen

fs/xfs/xfs_da_btree.c +0 -2

-	/* REFERENCED */
-	xfs_da_blkinfo_t *blkinfo;
 	xfs_da_args_t *args;
 	xfs_dablk_t child;
 	xfs_dabuf_t *bp;

@@ -742,15 +738,14 @@ xfs_da_root_join(xfs_da_state_t *state,
 	if (error)
 		return(error);

fs/xfs/xfs_da_btree.c xfs_da_root_join(xfs_da_state_t *state, · line 745

Quoted 10 lines

fs/xfs/xfs_da_btree.c +3 -4

 	ASSERT(bp != NULL);
-	blkinfo = bp->data;
 	if (be16_to_cpu(oldroot->hdr.level) == 1) {
-		ASSERT(be16_to_cpu(blkinfo->magic) == XFS_DIR2_LEAFN_MAGIC ||
-		       be16_to_cpu(blkinfo->magic) == XFS_ATTR_LEAF_MAGIC);
+		ASSERT(be16_to_cpu(bp->data->magic) == XFS_DIR2_LEAFN_MAGIC ||
+		       be16_to_cpu(bp->data->magic) == XFS_ATTR_LEAF_MAGIC);
 	} else {
-		ASSERT(be16_to_cpu(blkinfo->magic) == XFS_DA_NODE_MAGIC);
+		ASSERT(be16_to_cpu(bp->data->magic) == XFS_DA_NODE_MAGIC);

How can this work? bp->data is a void pointer.

Quoted 9 lines

fs/xfs/xfs_filestream.c +0 -1

@@ -174,7 +173,6 @@ _xfs_filestream_pick_ag(
 		/* Keep track of the AG with the most free blocks. */
 		if (pag->pagf_freeblks > maxfree) {
 			maxfree = pag->pagf_freeblks;
-			max_streams = atomic_read(&pag->pagf_fstrms);
 			max_ag = ag;
 		}

fs/xfs/xfs_filestream.c _xfs_filestream_pick_ag( · line 200

Quoted 5 lines

fs/xfs/xfs_filestream.c +0 -2

@@ -196,8 +194,6 @@ _xfs_filestream_pick_ag(
 		     (flags & XFS_PICK_LOWSPACE))) {
 
 			/* Break out, retaining the reference on the AG. */
-			free = pag->pagf_freeblks;
-			streams = atomic_read(&pag->pagf_fstrms);

These are used by the tracing code not yet converted to the new tracing infrastructure. I think I'll prefer to just add the new tracing code ASAP to make the warnings go away.

fs/xfs/xfs_filestream.c xfs_fstrm_free_func( · line 362

Quoted 9 lines

fs/xfs/xfs_filestream.c +2 -1

 			break;
 		}

@@ -364,7 +358,8 @@ xfs_fstrm_free_func(
 	/* Drop the reference taken on the AG when the item was added. */
 	ref = xfs_filestream_put_ag(ip->i_mount, item->ag);
 
-	ASSERT(ref >= 0);
+	if (ref < 0)
+		ASSERT(0);

We never use the return value of xfs_filestream_put_ag, so we an just make it return void (and move it to xfs_filestreams.c while at it).

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs

Andi Kleen

2010-06-14 08:57 UTC #2 lore raw

Quoted 13 lines · Christoph Hellwig

Quoted 12 lines · Andi Kleen

fs/xfs/xfs_da_btree.c +0 -4

-	/* REFERENCED */
-	xfs_da_blkinfo_t *blkinfo;
  	xfs_da_args_t *args;
  	xfs_dablk_t child;
  	xfs_dabuf_t *bp;

@@ -742,15 +738,14 @@ xfs_da_root_join(xfs_da_state_t *state,
  	if (error)
  		return(error);
  	ASSERT(bp != NULL);
-	blkinfo = bp->data;
  	if (be16_to_cpu(oldroot->hdr.level) == 1) {
-		ASSERT(be16_to_cpu(blkinfo->magic) == XFS_DIR2_LEAFN_MAGIC ||

Quoted 8 lines

fs/xfs/xfs_da_btree.c xfs_da_root_join(xfs_da_state_t *state, · line 745

Quoted 6 lines

fs/xfs/xfs_da_btree.c +3 -2

-		       be16_to_cpu(blkinfo->magic) == XFS_ATTR_LEAF_MAGIC);
+		ASSERT(be16_to_cpu(bp->data->magic) == XFS_DIR2_LEAFN_MAGIC ||
+		       be16_to_cpu(bp->data->magic) == XFS_ATTR_LEAF_MAGIC);
  	} else {
-		ASSERT(be16_to_cpu(blkinfo->magic) == XFS_DA_NODE_MAGIC);
+		ASSERT(be16_to_cpu(bp->data->magic) == XFS_DA_NODE_MAGIC);

How can this work? bp->data is a void pointer.

It compiled with CONFIG_XFS_DEBUG enabled. Do I need to set some other option to test this?

Anyways I don't plan to do any more heavy lifting on this patch, so if you need any other complicated changes someone else will have to do it.

-Andi

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs

Christoph Hellwig

2010-06-14 12:45 UTC #3 lore raw

Quoted 4 lines · Andi Kleen

Quoted 1 line
How can this work? bp->data is a void pointer.
It compiled with CONFIG_XFS_DEBUG enabled. Do I need to set some other option to test this?

It fails for me in a normal CONFIG_XFS_DEBUG build:

fs/xfs/xfs_da_btree.c: In function 'xfs_da_root_join':
fs/xfs/xfs_da_btree.c:737: warning: dereferencing 'void *' pointer fs/xfs/xfs_da_btree.c:737: error: request for member 'magic' in something not a structure or union fs/xfs/xfs_da_btree.c:737: warning: dereferencing 'void *' pointer

...

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs

Andi Kleen

author

2010-06-14 13:37 UTC #4 lore raw

Quoted 6 lines · Christoph Hellwig

Quoted 4 lines · Andi Kleen
Quoted 1 line
How can this work? bp->data is a void pointer.
It compiled with CONFIG_XFS_DEBUG enabled. Do I need to set some other option to test this?

Quoted 7 lines

It fails for me in a normal CONFIG_XFS_DEBUG build:
fs/xfs/xfs_da_btree.c: In function 'xfs_da_root_join':
fs/xfs/xfs_da_btree.c:737: warning: dereferencing 'void *' pointer fs/xfs/xfs_da_btree.c:737: error: request for member 'magic' in something not a structure or union fs/xfs/xfs_da_btree.c:737: warning: dereferencing 'void *' pointer

Hmm, it really compiled here (scratching head)

% grep XFS .config
CONFIG_XFS_FS=m
# CONFIG_XFS_QUOTA is not set
CONFIG_XFS_POSIX_ACL=y
# CONFIG_XFS_RT is not set
CONFIG_XFS_DEBUG=y
% make CC=gcc46 SUBDIRS=fs/xfs

  LD      fs/xfs/built-in.o
  CC [M]  fs/xfs/linux-2.6/xfs_trace.o
  CC [M]  fs/xfs/linux-2.6/xfs_acl.o
  CC [M]  fs/xfs/linux-2.6/xfs_stats.o
  CC [M]  fs/xfs/linux-2.6/xfs_sysctl.o
  CC [M]  fs/xfs/linux-2.6/xfs_ioctl32.o
  CC [M]  fs/xfs/xfs_alloc.o
  CC [M]  fs/xfs/xfs_alloc_btree.o
  CC [M]  fs/xfs/xfs_attr.o
  CC [M]  fs/xfs/xfs_attr_leaf.o
  CC [M]  fs/xfs/xfs_bit.o
  CC [M]  fs/xfs/xfs_bmap.o
  CC [M]  fs/xfs/xfs_bmap_btree.o
  CC [M]  fs/xfs/xfs_btree.o
  CC [M]  fs/xfs/xfs_buf_item.o
  CC [M]  fs/xfs/xfs_da_btree.o   <---------------------------------
  CC [M]  fs/xfs/xfs_dir2.o
  CC [M]  fs/xfs/xfs_dir2_block.o

fs/xfs/xfs_dir2_block.c: In function 'xfs_dir2_sf_to_block':
fs/xfs/xfs_dir2_block.c:1153:26: warning: array subscript is above array bounds [-Warray- bounds]
...

Do I need to set some other option?

-Andi

Signature · 6 lines

[email protected] -- Speaking for myself only.

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs

Andi Kleen

author

2010-06-14 16:28 UTC #5 lore raw

Quoted 9 lines · Andi Kleen

Quoted 8 lines · Christoph Hellwig
Quoted 4 lines · Andi Kleen
Quoted 1 line
How can this work? bp->data is a void pointer.
It compiled with CONFIG_XFS_DEBUG enabled. Do I need to set some other option to test this?
It fails for me in a normal CONFIG_XFS_DEBUG build:

Quoted 8 lines

Quoted 5 lines
fs/xfs/xfs_da_btree.c: In function 'xfs_da_root_join':
fs/xfs/xfs_da_btree.c:737: warning: dereferencing 'void *' pointer fs/xfs/xfs_da_btree.c:737: error: request for member 'magic' in something not a structure or union fs/xfs/xfs_da_btree.c:737: warning: dereferencing 'void *' pointer
Hmm, it really compiled here (scratching head)

Never mind, it seems SUBDIRS doesn't set the DEBUG flag correctly.

Anyways I just dropped those hunks and you'll have to live with the warnings if you don't want QASSERT.

Final version

-Andi

---

XFS: Fix gcc 4.6 set but not read and unused statement warnings v3

For my configuration, that is without quota or RT.

Mostly dead code removed I think (but needs additional review)

That is there were one or two bad error handling cases, but they were not easily fixable, with comments and I left the warnings in for those for you to remember.

e.g. if there is a ENOSPC down in xfs_trans.c while modifying the superblock it would not be handled.

Unused statements were mostly related to stub macros for disabled features like QUOTA or RT ALLOC. I replace those with inlines.

There were also some problems with variables used in ASSERT() I partly moved those into the ASSERT itself.

Cc [email protected]

Signed-off-by Andi Kleen <[email protected]>

---
 fs/xfs/linux-2.6/xfs_sync.c |    3 +++
 fs/xfs/xfs_alloc.c          |   10 +++-------
 fs/xfs/xfs_da_btree.c       |    6 ++----
 fs/xfs/xfs_dir2_block.c     |    6 +++---
 fs/xfs/xfs_filestream.c     |   16 +++++-----------
 fs/xfs/xfs_iget.c           |    3 ---
 fs/xfs/xfs_inode.c          |    4 ----
 fs/xfs/xfs_inode_item.c     |    7 ++-----
 fs/xfs/xfs_log.c            |    2 --
 fs/xfs/xfs_quota.h          |   14 ++++++++++----
 fs/xfs/xfs_trans.c          |    6 +++++-
 11 files changed, 33 insertions(+), 44 deletions(-)

fs/xfs/linux-2.6/xfs_sync.c +3 -0

Index: linux-2.6.35-rc3-gcc/fs/xfs/linux-2.6/xfs_sync.c
===================================================================
@@ -554,6 +554,9 @@ xfs_sync_worker(
 		xfs_log_force(mp, 0);
 		xfs_reclaim_inodes(mp, 0);
 		/* dgc: errors ignored here */
+		/* ak: yes and you'll get a warning for it now when you
+		 * upgrade compilers.
+		 */
 		error = xfs_qm_sync(mp, SYNC_TRYLOCK);
 		if (xfs_log_need_covered(mp))
 			error = xfs_commit_dummy_trans(mp, 0);

fs/xfs/xfs_da_btree.c +2 -4

Index: linux-2.6.35-rc3-gcc/fs/xfs/xfs_da_btree.c
===================================================================
@@ -581,16 +581,14 @@ xfs_da_node_add(xfs_da_state_t *state, x
 	xfs_da_intnode_t *node;
 	xfs_da_node_entry_t *btree;
 	int tmp;
-	xfs_mount_t *mp;
 
 	node = oldblk->bp->data;
-	mp = state->mp;
 	ASSERT(be16_to_cpu(node->hdr.info.magic) == XFS_DA_NODE_MAGIC);
 	ASSERT((oldblk->index >= 0) && (oldblk->index <= be16_to_cpu(node->hdr.count)));
 	ASSERT(newblk->blkno != 0);
 	if (state->args->whichfork == XFS_DATA_FORK)
-		ASSERT(newblk->blkno >= mp->m_dirleafblk &&
-		       newblk->blkno < mp->m_dirfreeblk);
+		ASSERT(newblk->blkno >= state->mp->m_dirleafblk &&
+		       newblk->blkno < state->mp->m_dirfreeblk);
 
 	/*
 	 * We may need to make some room before we insert the new node.

fs/xfs/xfs_dir2_block.c +3 -3

Index: linux-2.6.35-rc3-gcc/fs/xfs/xfs_dir2_block.c
===================================================================
@@ -1073,10 +1073,10 @@ xfs_dir2_sf_to_block(
 	 */
 
 	buf_len = dp->i_df.if_bytes;
-	buf = kmem_alloc(dp->i_df.if_bytes, KM_SLEEP);
+	buf = kmem_alloc(buf_len, KM_SLEEP);
 
-	memcpy(buf, sfp, dp->i_df.if_bytes);
-	xfs_idata_realloc(dp, -dp->i_df.if_bytes, XFS_DATA_FORK);
+	memcpy(buf, sfp, buf_len);
+	xfs_idata_realloc(dp, -buf_len, XFS_DATA_FORK);
 	dp->i_d.di_size = 0;
 	xfs_trans_log_inode(tp, dp, XFS_ILOG_CORE);
 	/*

fs/xfs/xfs_filestream.c +5 ~~-11~~

Index: linux-2.6.35-rc3-gcc/fs/xfs/xfs_filestream.c
===================================================================
@@ -140,9 +140,8 @@ _xfs_filestream_pick_ag(
 	int		flags,
 	xfs_extlen_t	minlen)
 {
-	int		streams, max_streams;
 	int		err, trylock, nscan;
-	xfs_extlen_t	longest, free, minfree, maxfree = 0;
+	xfs_extlen_t	longest, minfree, maxfree = 0;
 	xfs_agnumber_t	ag, max_ag = NULLAGNUMBER;
 	struct xfs_perag *pag;

@@ -174,7 +173,6 @@ _xfs_filestream_pick_ag(
 		/* Keep track of the AG with the most free blocks. */
 		if (pag->pagf_freeblks > maxfree) {
 			maxfree = pag->pagf_freeblks;
-			max_streams = atomic_read(&pag->pagf_fstrms);
 			max_ag = ag;
 		}

@@ -196,8 +194,6 @@ _xfs_filestream_pick_ag(
 		     (flags & XFS_PICK_LOWSPACE))) {
 
 			/* Break out, retaining the reference on the AG. */
-			free = pag->pagf_freeblks;
-			streams = atomic_read(&pag->pagf_fstrms);
 			xfs_perag_put(pag);
 			*agp = ag;
 			break;

@@ -234,8 +230,6 @@ next_ag:
 		if (max_ag != NULLAGNUMBER) {
 			xfs_filestream_get_ag(mp, max_ag);
 			TRACE_AG_PICK1(mp, max_ag, maxfree);
-			streams = max_streams;
-			free = maxfree;
 			*agp = max_ag;
 			break;
 		}

@@ -364,7 +358,8 @@ xfs_fstrm_free_func(
 	/* Drop the reference taken on the AG when the item was added. */
 	ref = xfs_filestream_put_ag(ip->i_mount, item->ag);
 
-	ASSERT(ref >= 0);
+	if (ref < 0)
+		ASSERT(0);
 	TRACE_FREE(ip->i_mount, ip, item->pip, item->ag,
 		xfs_filestream_peek_ag(ip->i_mount, item->ag));

@@ -465,7 +460,6 @@ xfs_filestream_lookup_ag(
 	xfs_mru_cache_t	*cache;
 	fstrm_item_t	*item;
 	xfs_agnumber_t	ag;
-	int		ref;
 
 	if (!(ip->i_d.di_mode & (S_IFREG | S_IFDIR))) {
 		ASSERT(0);

@@ -481,10 +475,10 @@ xfs_filestream_lookup_ag(
 
 	ASSERT(ip == item->ip);
 	ag = item->ag;
-	ref = xfs_filestream_peek_ag(ip->i_mount, ag);
 	xfs_mru_cache_done(cache);
 
-	TRACE_LOOKUP(ip->i_mount, ip, item->pip, ag, ref);
+	TRACE_LOOKUP(ip->i_mount, ip, item->pip, ag,
+			xfs_filestream_peek_ag(ip->i_mount, ag));
 	return ag;
 }

fs/xfs/xfs_iget.c +0 -3

Index: linux-2.6.35-rc3-gcc/fs/xfs/xfs_iget.c
===================================================================
@@ -265,7 +265,6 @@ xfs_iget_cache_miss(
 {
 	struct xfs_inode	*ip;
 	int			error;
-	unsigned long		first_index, mask;
 	xfs_agino_t		agino = XFS_INO_TO_AGINO(mp, ino);
 
 	ip = xfs_inode_alloc(mp, ino);

@@ -302,8 +301,6 @@ xfs_iget_cache_miss(
 			BUG();
 	}
 
-	mask = ~(((XFS_INODE_CLUSTER_SIZE(mp) >> mp->m_sb.sb_inodelog)) - 1);
-	first_index = agino & mask;
 	write_lock(&pag->pag_ici_lock);
 
 	/* insert the new inode */

fs/xfs/xfs_inode.c +0 -4

Index: linux-2.6.35-rc3-gcc/fs/xfs/xfs_inode.c
===================================================================
@@ -925,7 +925,6 @@ xfs_iread_extents(
 	int		error;
 	xfs_ifork_t	*ifp;
 	xfs_extnum_t	nextents;
-	size_t		size;
 
 	if (unlikely(XFS_IFORK_FORMAT(ip, whichfork) != XFS_DINODE_FMT_BTREE)) {
 		XFS_ERROR_REPORT("xfs_iread_extents", XFS_ERRLEVEL_LOW,

@@ -933,7 +932,6 @@ xfs_iread_extents(
 		return XFS_ERROR(EFSCORRUPTED);
 	}
 	nextents = XFS_IFORK_NEXTENTS(ip, whichfork);
-	size = nextents * sizeof(xfs_bmbt_rec_t);
 	ifp = XFS_IFORK_PTR(ip, whichfork);
 
 	/*

@@ -3517,13 +3515,11 @@ xfs_iext_remove_indirect(
 	xfs_extnum_t	ext_diff;	/* extents to remove in current list */
 	xfs_extnum_t	nex1;		/* number of extents before idx */
 	xfs_extnum_t	nex2;		/* extents after idx + count */
-	int		nlists;		/* entries in indirection array */
 	int		page_idx = idx;	/* index in target extent list */
 
 	ASSERT(ifp->if_flags & XFS_IFEXTIREC);
 	erp = xfs_iext_idx_to_irec(ifp,  &page_idx, &erp_idx, 0);
 	ASSERT(erp != NULL);
-	nlists = ifp->if_real_bytes / XFS_IEXT_BUFSZ;
 	nex1 = page_idx;
 	ext_cnt = count;
 	while (ext_cnt) {

fs/xfs/xfs_inode_item.c +2 -5

Index: linux-2.6.35-rc3-gcc/fs/xfs/xfs_inode_item.c
===================================================================
@@ -323,9 +323,8 @@ xfs_inode_item_format(
 			ASSERT(ip->i_df.if_u1.if_extents != NULL);
 			ASSERT(ip->i_d.di_nextents > 0);
 			ASSERT(iip->ili_extents_buf == NULL);
-			nrecs = ip->i_df.if_bytes /
-				(uint)sizeof(xfs_bmbt_rec_t);
-			ASSERT(nrecs > 0);
+			ASSERT((ip->i_df.if_bytes /
+				(uint)sizeof(xfs_bmbt_rec_t)) > 0);
 #ifdef XFS_NATIVE_HOST
 			if (nrecs == ip->i_d.di_nextents) {
 				/*

@@ -957,10 +956,8 @@ xfs_iflush_abort(
 	xfs_inode_t		*ip)
 {
 	xfs_inode_log_item_t	*iip = ip->i_itemp;
-	xfs_mount_t		*mp;
 
 	iip = ip->i_itemp;
-	mp = ip->i_mount;
 	if (iip) {
 		struct xfs_ail	*ailp = iip->ili_item.li_ailp;
 		if (iip->ili_item.li_flags & XFS_LI_IN_AIL) {

fs/xfs/xfs_log.c +0 -2

Index: linux-2.6.35-rc3-gcc/fs/xfs/xfs_log.c
===================================================================
@@ -1047,7 +1047,6 @@ xlog_alloc_log(xfs_mount_t	*mp,
 	xlog_in_core_t		*iclog, *prev_iclog=NULL;
 	xfs_buf_t		*bp;
 	int			i;
-	int			iclogsize;
 	int			error = ENOMEM;
 	uint			log2_size = 0;

@@ -1127,7 +1126,6 @@ xlog_alloc_log(xfs_mount_t	*mp,
 	 * with different amounts of memory.  See the definition of
 	 * xlog_in_core_t in xfs_log_priv.h for details.
 	 */
-	iclogsize = log->l_iclog_size;
 	ASSERT(log->l_iclog_size >= 4096);
 	for (i=0; i < log->l_iclog_bufs; i++) {
 		*iclogp = kmem_zalloc(sizeof(xlog_in_core_t), KM_MAYFAIL);

fs/xfs/xfs_quota.h +10 -4

Index: linux-2.6.35-rc3-gcc/fs/xfs/xfs_quota.h
===================================================================
@@ -346,7 +346,13 @@ xfs_qm_vop_dqalloc(struct xfs_inode *ip,
 #define xfs_trans_mod_dquot_byino(tp, ip, fields, delta)
 #define xfs_trans_apply_dquot_deltas(tp)
 #define xfs_trans_unreserve_and_mod_dquots(tp)
-#define xfs_trans_reserve_quota_nblks(tp, ip, nblks, ninos, flags)	(0)
+
+static inline int xfs_trans_reserve_quota_nblks(struct xfs_trans *t,
+		struct xfs_inode *i, long a, long b, uint c)
+{
+	return 0;
+}
+
 #define xfs_trans_reserve_quota_bydquots(tp, mp, u, g, nb, ni, fl)	(0)
 #define xfs_qm_vop_create_dqattach(tp, ip, u, g)
 #define xfs_qm_vop_rename_dqattach(it)					(0)

@@ -355,13 +361,13 @@ xfs_qm_vop_dqalloc(struct xfs_inode *ip,
 #define xfs_qm_dqattach(ip, fl)						(0)
 #define xfs_qm_dqattach_locked(ip, fl)					(0)
 #define xfs_qm_dqdetach(ip)
-#define xfs_qm_dqrele(d)
+static inline void xfs_qm_dqrele(struct xfs_dquot *d) {}
 #define xfs_qm_statvfs(ip, s)
-#define xfs_qm_sync(mp, fl)						(0)
+static inline int xfs_qm_sync(struct xfs_mount *m, int i) { return 0; }
 #define xfs_qm_newmount(mp, a, b)					(0)
 #define xfs_qm_mount_quotas(mp)
 #define xfs_qm_unmount(mp)
-#define xfs_qm_unmount_quotas(mp)					(0)
+static inline void xfs_qm_unmount_quotas(struct xfs_mount *m) {}
 #endif /* CONFIG_XFS_QUOTA */
 
 #define xfs_trans_unreserve_quota_nblks(tp, ip, nblks, ninos, flags) \

fs/xfs/xfs_trans.c +5 -1

Index: linux-2.6.35-rc3-gcc/fs/xfs/xfs_trans.c
===================================================================
@@ -1119,7 +1119,11 @@ xfs_trans_unreserve_and_mod_sb(
 	if (msbp > msb) {
 		error = xfs_mod_incore_sb_batch(tp->t_mountp, msb,
 			(uint)(msbp - msb), rsvd);
-		ASSERT(error == 0);
+		/*
+	 	 * When an error happens here it's a race that's supposed
+		 * to be fixed. Check for this unconditionally.
+		 */
+		BUG_ON(error != 0);
 	}
 }

fs/xfs/xfs_alloc.c +3 -7

Index: linux-2.6.35-rc3-gcc/fs/xfs/xfs_alloc.c
===================================================================
@@ -688,8 +688,6 @@ xfs_alloc_ag_vextent_near(
 	xfs_agblock_t	ltbno;		/* start bno of left side entry */
 	xfs_agblock_t	ltbnoa;		/* aligned ... */
 	xfs_extlen_t	ltdiff;		/* difference to left side entry */
-	/*REFERENCED*/
-	xfs_agblock_t	ltend;		/* end bno of left side entry */
 	xfs_extlen_t	ltlen;		/* length of left side entry */
 	xfs_extlen_t	ltlena;		/* aligned ... */
 	xfs_agblock_t	ltnew;		/* useful start bno of left side */

@@ -814,8 +812,7 @@ xfs_alloc_ag_vextent_near(
 		if ((error = xfs_alloc_get_rec(cnt_cur, &ltbno, &ltlen, &i)))
 			goto error0;
 		XFS_WANT_CORRUPTED_GOTO(i == 1, error0);
-		ltend = ltbno + ltlen;
-		ASSERT(ltend <= be32_to_cpu(XFS_BUF_TO_AGF(args->agbp)->agf_length));
+		ASSERT(ltbno + ltlen <= be32_to_cpu(XFS_BUF_TO_AGF(args->agbp)->agf_length));
 		args->len = blen;
 		if (!xfs_alloc_fix_minleft(args)) {
 			xfs_btree_del_cursor(cnt_cur, XFS_BTREE_NOERROR);

@@ -828,7 +825,7 @@ xfs_alloc_ag_vextent_near(
 		 */
 		args->agbno = bnew;
 		ASSERT(bnew >= ltbno);
-		ASSERT(bnew + blen <= ltend);
+		ASSERT(bnew + blen <= ltbno + ltlen);
 		/*
 		 * Set up a cursor for the by-bno tree.
 		 */

@@ -1157,7 +1154,6 @@ xfs_alloc_ag_vextent_near(
 	/*
 	 * Fix up the length and compute the useful address.
 	 */
-	ltend = ltbno + ltlen;
 	args->len = XFS_EXTLEN_MIN(ltlena, args->maxlen);
 	xfs_alloc_fix_len(args);
 	if (!xfs_alloc_fix_minleft(args)) {

@@ -1170,7 +1166,7 @@ xfs_alloc_ag_vextent_near(
 	(void)xfs_alloc_compute_diff(args->agbno, rlen, args->alignment, ltbno,
 		ltlen, &ltnew);
 	ASSERT(ltnew >= ltbno);
-	ASSERT(ltnew + rlen <= ltend);
+	ASSERT(ltnew + rlen <= ltbno + ltlen);
 	ASSERT(ltnew + rlen <= be32_to_cpu(XFS_BUF_TO_AGF(args->agbp)->agf_length));
 	args->agbno = ltnew;
 	if ((error = xfs_alloc_fixup_trees(cnt_cur, bno_cur_lt, ltbno, ltlen,

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs

Dave Chinner

2010-06-15 00:52 UTC #6 lore raw

Quoted 17 lines · Andi Kleen

Quoted 16 lines · Andi Kleen
Quoted 13 lines · Christoph Hellwig
Quoted 4 lines · Andi Kleen
Quoted 1 line
How can this work? bp->data is a void pointer.
It compiled with CONFIG_XFS_DEBUG enabled. Do I need to set some other option to test this?
It fails for me in a normal CONFIG_XFS_DEBUG build:
fs/xfs/xfs_da_btree.c: In function 'xfs_da_root_join':
fs/xfs/xfs_da_btree.c:737: warning: dereferencing 'void *' pointer fs/xfs/xfs_da_btree.c:737: error: request for member 'magic' in something not a structure or union fs/xfs/xfs_da_btree.c:737: warning: dereferencing 'void *' pointer

Quoted 6 lines

Quoted 1 line
Hmm, it really compiled here (scratching head)
Never mind, it seems SUBDIRS doesn't set the DEBUG flag correctly.
Anyways I just dropped those hunks and you'll have to live with the warnings if you don't want QASSERT.

I'm still not happy with it.

We'll commit a modified version of one of your patches into the XFS tree as you seem unable to accept that changes might be necessary to get your code to pass review.

Cheers,

Dave.

Signature · 7 lines

Dave Chinner
[email protected]

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs

Andi Kleen

author

2010-06-15 07:22 UTC #7 lore raw

Quoted 1 line

I'm still not happy with it.

With the BUG_ON?

Quoted 4 lines

We'll commit a modified version of one of your patches into the XFS tree as you seem unable to accept that changes might be necessary to get your code to pass review.

Sure modify however you like as long as you fix the warnings.

Also it was good to have review on the cases it warned about, luckily there were no real bugs found.

[BTW I should add there were some more mostly from array overflows. I have not investigated in detail, but I suspect they were harmless]

My goal was also mostly to get my build reasonably warning free, but if I can't get this patch in I can also just disable the warning for fs/xfs.

-Andi

Signature · 6 lines

[email protected] -- Speaking for myself only.

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs

Eric Sandeen

2010-06-16 02:45 UTC #8 lore raw

In reply to Andi Kleen

Quoted 14 lines · Andi Kleen

Quoted 13 lines · Andi Kleen
Quoted 11 lines · Christoph Hellwig
Quoted 3 lines · Andi Kleen
Quoted 1 line
How can this work? bp->data is a void pointer.
It compiled with CONFIG_XFS_DEBUG enabled. Do I need to set some other option to test this?
It fails for me in a normal CONFIG_XFS_DEBUG build:
fs/xfs/xfs_da_btree.c: In function 'xfs_da_root_join':
fs/xfs/xfs_da_btree.c:737: warning: dereferencing 'void *' pointer fs/xfs/xfs_da_btree.c:737: error: request for member 'magic' in something not a structure or union fs/xfs/xfs_da_btree.c:737: warning: dereferencing 'void *' pointer

Quoted 3 lines

Quoted 1 line
Hmm, it really compiled here (scratching head)
Never mind, it seems SUBDIRS doesn't set the DEBUG flag correctly.

really?

$ make SUBDIRS=fs/xfs
  CC [M]  fs/xfs/linux-2.6/xfs_super.o
fs/xfs/linux-2.6/xfs_super.c:73:2: error: #error DEBUG
make[1]: *** [fs/xfs/linux-2.6/xfs_super.o] Error 1

$ grep -A3 XFS_DEBUG fs/xfs/linux-2.6/xfs_super.c 
#ifdef CONFIG_XFS_DEBUG
#error DEBUG
#endif

(above just for testing)

Offtopic I guess, but it sure seems to set it for me.

-Eric

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs

Andi Kleen

2010-06-16 07:31 UTC #9 lore raw

Quoted 8 lines

really?

$ make SUBDIRS=fs/xfs
   CC [M]  fs/xfs/linux-2.6/xfs_super.o
fs/xfs/linux-2.6/xfs_super.c:73:2: error: #error DEBUG
make[1]: *** [fs/xfs/linux-2.6/xfs_super.o] Error 1

Quoted 8 lines

$ grep -A3 XFS_DEBUG fs/xfs/linux-2.6/xfs_super.c
#ifdef CONFIG_XFS_DEBUG
#error DEBUG
#endif
(above just for testing)
Offtopic I guess, but it sure seems to set it for me.

You did a full build with XFS_DEBUG before without SUBDIRS, right?

Kbuild puts the options into special .cmd files and I think those only get rebuild on a full build, not on a SUBDIRS build. So it never picked up the changed option for me.

Anyways that's the current theory, haven't retested that.

-Andi

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs