[PATCH 0/4 V2] Remove a few macros

[PATCH 0/4 V2] Remove a few macros

[Carlos Maiolino]

Hi,

this is a second version of the patchset aimed to clean up some old macros from
xfsprogs, as the objective to bring xfs_buf handling code closer to kernel code.

It fixes some pointer castings Dave mentioned on the V1 patch, also, it removes
a few unneeded castings I spotted while reviewing the ones Dave mentioned.

Cheers

Carlos Maiolino (4):
  Get rid of XFS_BUF_PTR() macro
  Get rid of XFS_BUF_TARGET() macro
  get rid of XFS_BUF_COUNT() macro
  Get rid of XFS_BUF_SET_COUNT() macro

 libxfs/libxfs_io.h        |  6 +-----
 libxfs/logitem.c          |  2 +-
 libxfs/rdwr.c             | 10 +++++-----
 libxfs/trans.c            |  2 +-
 libxlog/xfs_log_recover.c |  2 +-
 logprint/log_print_all.c  |  4 ++--
 mkfs/proto.c              |  8 ++++----
 mkfs/xfs_mkfs.c           | 14 +++++++-------
 repair/agheader.c         |  8 ++++----
 repair/attr_repair.c      |  4 ++--
 repair/dino_chunks.c      |  2 +-
 repair/phase6.c           |  4 ++--
 repair/prefetch.c         |  4 ++--
 13 files changed, 33 insertions(+), 37 deletions(-)

-- 
2.14.3

[PATCH 1/4] Get rid of XFS_BUF_PTR() macro

[Carlos Maiolino]

Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com>
---
V2:
	Fix casting

 libxfs/libxfs_io.h       |  1 -
 libxfs/rdwr.c            | 10 +++++-----
 logprint/log_print_all.c |  4 ++--
 mkfs/proto.c             |  5 +++--
 mkfs/xfs_mkfs.c          | 14 +++++++-------
 repair/agheader.c        |  8 ++++----
 repair/phase6.c          |  4 ++--
 repair/prefetch.c        |  2 +-
 8 files changed, 24 insertions(+), 24 deletions(-)

diff --git a/libxfs/libxfs_io.h b/libxfs/libxfs_io.h
index 6308a742..a1c62071 100644
--- a/libxfs/libxfs_io.h
+++ b/libxfs/libxfs_io.h
@@ -97,7 +97,6 @@ enum xfs_buf_flags_t {	/* b_flags bits */
 
 #define XFS_BUF_DADDR_NULL		((xfs_daddr_t) (-1LL))
 
-#define XFS_BUF_PTR(bp)			((char *)(bp)->b_addr)
 #define xfs_buf_offset(bp, offset)	((bp)->b_addr + (offset))
 #define XFS_BUF_ADDR(bp)		((bp)->b_bn)
 #define XFS_BUF_SIZE(bp)		((bp)->b_bcount)
diff --git a/libxfs/rdwr.c b/libxfs/rdwr.c
index 3c5def29..9acdb98c 100644
--- a/libxfs/rdwr.c
+++ b/libxfs/rdwr.c
@@ -143,7 +143,7 @@ static char *next(
 	struct xfs_buf	*buf = (struct xfs_buf *)private;
 
 	if (buf &&
-	    (XFS_BUF_COUNT(buf) < (int)(ptr - XFS_BUF_PTR(buf)) + offset))
+	    (XFS_BUF_COUNT(buf) < (int)(ptr - (char *)buf->b_addr) + offset))
 		abort();
 
 	return ptr + offset;
@@ -204,7 +204,7 @@ libxfs_log_clear(
 	ptr = dptr;
 	if (btp) {
 		bp = libxfs_getbufr(btp, start, len);
-		ptr = XFS_BUF_PTR(bp);
+		ptr = (char *)bp->b_addr;
 	}
 	libxfs_log_header(ptr, fs_uuid, version, sunit, fmt, lsn, tail_lsn,
 			  next, bp);
@@ -252,7 +252,7 @@ libxfs_log_clear(
 		ptr = dptr;
 		if (btp) {
 			bp = libxfs_getbufr(btp, blk, len);
-			ptr = XFS_BUF_PTR(bp);
+			ptr = (char *)bp->b_addr;
 		}
 		/*
 		 * Note: pass the full buffer length as the sunit to initialize
@@ -1026,7 +1026,7 @@ libxfs_readbufr_map(struct xfs_buftarg *btp, struct xfs_buf *bp, int flags)
 {
 	int	fd;
 	int	error = 0;
-	char	*buf;
+	void	*buf;
 	int	i;
 
 	fd = libxfs_device_to_fd(btp->dev);
@@ -1147,7 +1147,7 @@ libxfs_writebufr(xfs_buf_t *bp)
 				    LIBXFS_BBTOOFF64(bp->b_bn), bp->b_flags);
 	} else {
 		int	i;
-		char	*buf = bp->b_addr;
+		void	*buf = bp->b_addr;
 
 		for (i = 0; i < bp->b_nmaps; i++) {
 			off64_t	offset = LIBXFS_BBTOOFF64(bp->b_maps[i].bm_bn);
diff --git a/logprint/log_print_all.c b/logprint/log_print_all.c
index cdaf77bf..4eaf184f 100644
--- a/logprint/log_print_all.c
+++ b/logprint/log_print_all.c
@@ -39,10 +39,10 @@ xlog_print_find_oldest(
 	first_blk = 0;		/* read first block */
 	bp = xlog_get_bp(log, 1);
 	xlog_bread_noalign(log, 0, 1, bp);
-	first_half_cycle = xlog_get_cycle(XFS_BUF_PTR(bp));
+	first_half_cycle = xlog_get_cycle((char *)bp->b_addr);
 	*last_blk = log->l_logBBsize-1;	/* read last block */
 	xlog_bread_noalign(log, *last_blk, 1, bp);
-	last_half_cycle = xlog_get_cycle(XFS_BUF_PTR(bp));
+	last_half_cycle = xlog_get_cycle((char *)bp->b_addr);
 	ASSERT(last_half_cycle != 0);
 
 	if (first_half_cycle == last_half_cycle) /* all cycle nos are same */
diff --git a/mkfs/proto.c b/mkfs/proto.c
index bc383458..15545ad1 100644
--- a/mkfs/proto.c
+++ b/mkfs/proto.c
@@ -275,9 +275,10 @@ newfile(
 		d = XFS_FSB_TO_DADDR(mp, map.br_startblock);
 		bp = libxfs_trans_get_buf(logit ? tp : 0, mp->m_dev, d,
 			nb << mp->m_blkbb_log, 0);
-		memmove(XFS_BUF_PTR(bp), buf, len);
+		memmove(bp->b_addr, buf, len);
 		if (len < XFS_BUF_COUNT(bp))
-			memset(XFS_BUF_PTR(bp) + len, 0, XFS_BUF_COUNT(bp) - len);
+			memset((char *)bp->b_addr + len, 0,
+			       XFS_BUF_COUNT(bp) - len);
 		if (logit)
 			libxfs_trans_log_buf(tp, bp, 0, XFS_BUF_COUNT(bp) - 1);
 		else
diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c
index f973b6bc..5467b901 100644
--- a/mkfs/xfs_mkfs.c
+++ b/mkfs/xfs_mkfs.c
@@ -3299,7 +3299,7 @@ prepare_devices(
 	 */
 	buf = libxfs_getbuf(mp->m_ddev_targp, (xi->dsize - whack_blks),
 			    whack_blks);
-	memset(XFS_BUF_PTR(buf), 0, WHACK_SIZE);
+	memset(buf->b_addr, 0, WHACK_SIZE);
 	libxfs_writebuf(buf, LIBXFS_EXIT_ON_FAILURE);
 	libxfs_purgebuf(buf);
 
@@ -3310,15 +3310,15 @@ prepare_devices(
 	 * ext[2,3] and reiserfs (64k) - and hopefully all else.
 	 */
 	buf = libxfs_getbuf(mp->m_ddev_targp, 0, whack_blks);
-	memset(XFS_BUF_PTR(buf), 0, WHACK_SIZE);
+	memset(buf->b_addr, 0, WHACK_SIZE);
 	libxfs_writebuf(buf, LIBXFS_EXIT_ON_FAILURE);
 	libxfs_purgebuf(buf);
 
 	/* OK, now write the superblock... */
 	buf = libxfs_getbuf(mp->m_ddev_targp, XFS_SB_DADDR, XFS_FSS_TO_BB(mp, 1));
 	buf->b_ops = &xfs_sb_buf_ops;
-	memset(XFS_BUF_PTR(buf), 0, cfg->sectorsize);
-	libxfs_sb_to_disk((void *)XFS_BUF_PTR(buf), sbp);
+	memset(buf->b_addr, 0, cfg->sectorsize);
+	libxfs_sb_to_disk(buf->b_addr, sbp);
 	libxfs_writebuf(buf, LIBXFS_EXIT_ON_FAILURE);
 	libxfs_purgebuf(buf);
 
@@ -3338,7 +3338,7 @@ prepare_devices(
 		buf = libxfs_getbuf(mp->m_rtdev_targp,
 				    XFS_FSB_TO_BB(mp, cfg->rtblocks - 1LL),
 				    BTOBB(cfg->blocksize));
-		memset(XFS_BUF_PTR(buf), 0, cfg->blocksize);
+		memset(buf->b_addr, 0, cfg->blocksize);
 		libxfs_writebuf(buf, LIBXFS_EXIT_ON_FAILURE);
 		libxfs_purgebuf(buf);
 	}
@@ -3382,8 +3382,8 @@ initialise_ag_headers(
 			XFS_AG_DADDR(mp, agno, XFS_SB_DADDR),
 			XFS_FSS_TO_BB(mp, 1));
 	buf->b_ops = &xfs_sb_buf_ops;
-	memset(XFS_BUF_PTR(buf), 0, cfg->sectorsize);
-	libxfs_sb_to_disk((void *)XFS_BUF_PTR(buf), sbp);
+	memset(buf->b_addr, 0, cfg->sectorsize);
+	libxfs_sb_to_disk(buf->b_addr, sbp);
 	libxfs_writebuf(buf, LIBXFS_EXIT_ON_FAILURE);
 
 	/*
diff --git a/repair/agheader.c b/repair/agheader.c
index cce376f2..1f6c82f3 100644
--- a/repair/agheader.c
+++ b/repair/agheader.c
@@ -290,8 +290,8 @@ secondary_sb_whack(
 			+ sizeof(sb->sb_dirblklog);
 
 	/* Check the buffer we read from disk for garbage outside size */
-	for (ip = XFS_BUF_PTR(sbuf) + size;
-	     ip < XFS_BUF_PTR(sbuf) + mp->m_sb.sb_sectsize;
+	for (ip = (char *)sbuf->b_addr + size;
+	     ip < (char *)sbuf->b_addr + mp->m_sb.sb_sectsize;
 	     ip++)  {
 		if (*ip)  {
 			do_bzero = 1;
@@ -314,7 +314,7 @@ secondary_sb_whack(
 			memcpy(&tmpuuid, &sb->sb_meta_uuid, sizeof(uuid_t));
 			memset((void *)((intptr_t)sb + size), 0,
 				mp->m_sb.sb_sectsize - size);
-			memset(XFS_BUF_PTR(sbuf) + size, 0,
+			memset((char *)sbuf->b_addr + size, 0,
 				mp->m_sb.sb_sectsize - size);
 			/* Preserve meta_uuid so we don't fail uuid checks */
 			memcpy(&sb->sb_meta_uuid, &tmpuuid, sizeof(uuid_t));
@@ -486,7 +486,7 @@ verify_set_agheader(xfs_mount_t *mp, xfs_buf_t *sbuf, xfs_sb_t *sb,
 	int status = XR_OK;
 	int status_sb = XR_OK;
 
-	status = verify_sb(sbuf->b_addr, sb, (i == 0));
+	status = verify_sb((char *)sbuf->b_addr, sb, (i == 0));
 
 	if (status != XR_OK)  {
 		do_warn(_("bad on-disk superblock %d - %s\n"),
diff --git a/repair/phase6.c b/repair/phase6.c
index 1a398aa1..65d2fd9e 100644
--- a/repair/phase6.c
+++ b/repair/phase6.c
@@ -632,7 +632,7 @@ _("can't access block %" PRIu64 " (fsbno %" PRIu64 ") of realtime bitmap inode %
 			return(1);
 		}
 
-		memmove(XFS_BUF_PTR(bp), bmp, mp->m_sb.sb_blocksize);
+		memmove(bp->b_addr, bmp, mp->m_sb.sb_blocksize);
 
 		libxfs_trans_log_buf(tp, bp, 0, mp->m_sb.sb_blocksize - 1);
 
@@ -704,7 +704,7 @@ _("can't access block %" PRIu64 " (fsbno %" PRIu64 ") of realtime summary inode
 			return(1);
 		}
 
-		memmove(XFS_BUF_PTR(bp), smp, mp->m_sb.sb_blocksize);
+		memmove(bp->b_addr, smp, mp->m_sb.sb_blocksize);
 
 		libxfs_trans_log_buf(tp, bp, 0, mp->m_sb.sb_blocksize - 1);
 
diff --git a/repair/prefetch.c b/repair/prefetch.c
index 9c68e35c..0783d225 100644
--- a/repair/prefetch.c
+++ b/repair/prefetch.c
@@ -591,7 +591,7 @@ pf_batch_read(
 				size = XFS_BUF_SIZE(bplist[i]);
 				if (len < size)
 					break;
-				memcpy(XFS_BUF_PTR(bplist[i]), pbuf, size);
+				memcpy(bplist[i]->b_addr, pbuf, size);
 				bplist[i]->b_flags |= (LIBXFS_B_UPTODATE |
 						       LIBXFS_B_UNCHECKED);
 				len -= size;
-- 
2.14.3

[PATCH 2/4] Get rid of XFS_BUF_TARGET() macro

[Carlos Maiolino]

This macro is unused

Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com>
---
 libxfs/libxfs_io.h | 1 -
 1 file changed, 1 deletion(-)

diff --git a/libxfs/libxfs_io.h b/libxfs/libxfs_io.h
index a1c62071..04bbc9b7 100644
--- a/libxfs/libxfs_io.h
+++ b/libxfs/libxfs_io.h
@@ -101,7 +101,6 @@ enum xfs_buf_flags_t {	/* b_flags bits */
 #define XFS_BUF_ADDR(bp)		((bp)->b_bn)
 #define XFS_BUF_SIZE(bp)		((bp)->b_bcount)
 #define XFS_BUF_COUNT(bp)		((bp)->b_bcount)
-#define XFS_BUF_TARGET(bp)		((bp)->b_dev)
 #define XFS_BUF_SET_PTR(bp,p,cnt)	({	\
 	(bp)->b_addr = (char *)(p);		\
 	XFS_BUF_SET_COUNT(bp,cnt);		\
-- 
2.14.3

[PATCH 3/4] get rid of XFS_BUF_COUNT() macro

[Carlos Maiolino]

Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com>
---
V2:
	Fix conflicts with changes on patch 1

 libxfs/libxfs_io.h   | 1 -
 libxfs/logitem.c     | 2 +-
 libxfs/rdwr.c        | 2 +-
 libxfs/trans.c       | 2 +-
 mkfs/proto.c         | 7 +++----
 repair/attr_repair.c | 4 ++--
 repair/dino_chunks.c | 2 +-
 repair/prefetch.c    | 2 +-
 8 files changed, 10 insertions(+), 12 deletions(-)

diff --git a/libxfs/libxfs_io.h b/libxfs/libxfs_io.h
index 04bbc9b7..f429d667 100644
--- a/libxfs/libxfs_io.h
+++ b/libxfs/libxfs_io.h
@@ -100,7 +100,6 @@ enum xfs_buf_flags_t {	/* b_flags bits */
 #define xfs_buf_offset(bp, offset)	((bp)->b_addr + (offset))
 #define XFS_BUF_ADDR(bp)		((bp)->b_bn)
 #define XFS_BUF_SIZE(bp)		((bp)->b_bcount)
-#define XFS_BUF_COUNT(bp)		((bp)->b_bcount)
 #define XFS_BUF_SET_PTR(bp,p,cnt)	({	\
 	(bp)->b_addr = (char *)(p);		\
 	XFS_BUF_SET_COUNT(bp,cnt);		\
diff --git a/libxfs/logitem.c b/libxfs/logitem.c
index 39ac1923..afe1570c 100644
--- a/libxfs/logitem.c
+++ b/libxfs/logitem.c
@@ -120,7 +120,7 @@ xfs_buf_item_init(
 	bip->bli_buf = bp;
 	bip->bli_format.blf_type = XFS_LI_BUF;
 	bip->bli_format.blf_blkno = (int64_t)XFS_BUF_ADDR(bp);
-	bip->bli_format.blf_len = (unsigned short)BTOBB(XFS_BUF_COUNT(bp));
+	bip->bli_format.blf_len = (unsigned short)BTOBB(bp->b_bcount);
 	bp->b_log_item = bip;
 }
 
diff --git a/libxfs/rdwr.c b/libxfs/rdwr.c
index 9acdb98c..60880de4 100644
--- a/libxfs/rdwr.c
+++ b/libxfs/rdwr.c
@@ -143,7 +143,7 @@ static char *next(
 	struct xfs_buf	*buf = (struct xfs_buf *)private;
 
 	if (buf &&
-	    (XFS_BUF_COUNT(buf) < (int)(ptr - (char *)buf->b_addr) + offset))
+	    (buf->b_bcount < (int)(ptr - (char *)buf->b_addr) + offset))
 		abort();
 
 	return ptr + offset;
diff --git a/libxfs/trans.c b/libxfs/trans.c
index 0e7b7ae0..3e80e0cb 100644
--- a/libxfs/trans.c
+++ b/libxfs/trans.c
@@ -402,7 +402,7 @@ libxfs_trans_log_buf(
 {
 	struct xfs_buf_log_item	*bip = bp->b_log_item;
 
-	ASSERT((first <= last) && (last < XFS_BUF_COUNT(bp)));
+	ASSERT((first <= last) && (last < bp->b_bcount));
 
 	xfs_trans_dirty_buf(tp, bp);
 	xfs_buf_item_log(bip, first, last);
diff --git a/mkfs/proto.c b/mkfs/proto.c
index 15545ad1..c925edb2 100644
--- a/mkfs/proto.c
+++ b/mkfs/proto.c
@@ -276,11 +276,10 @@ newfile(
 		bp = libxfs_trans_get_buf(logit ? tp : 0, mp->m_dev, d,
 			nb << mp->m_blkbb_log, 0);
 		memmove(bp->b_addr, buf, len);
-		if (len < XFS_BUF_COUNT(bp))
-			memset((char *)bp->b_addr + len, 0,
-			       XFS_BUF_COUNT(bp) - len);
+		if (len < bp->b_bcount)
+			memset((char *)bp->b_addr + len, 0, bp->b_bcount - len);
 		if (logit)
-			libxfs_trans_log_buf(tp, bp, 0, XFS_BUF_COUNT(bp) - 1);
+			libxfs_trans_log_buf(tp, bp, 0, bp->b_bcount - 1);
 		else
 			libxfs_writebuf(bp, LIBXFS_EXIT_ON_FAILURE);
 	}
diff --git a/repair/attr_repair.c b/repair/attr_repair.c
index 8b1b8a75..39569ebe 100644
--- a/repair/attr_repair.c
+++ b/repair/attr_repair.c
@@ -437,9 +437,9 @@ rmtval_get(xfs_mount_t *mp, xfs_ino_t ino, blkmap_t *blkmap,
 			break;
 		}
 
-		ASSERT(mp->m_sb.sb_blocksize == XFS_BUF_COUNT(bp));
+		ASSERT(mp->m_sb.sb_blocksize == bp->b_bcount);
 
-		length = MIN(XFS_BUF_COUNT(bp) - hdrsize, valuelen - amountdone);
+		length = MIN(bp->b_bcount - hdrsize, valuelen - amountdone);
 		memmove(value, bp->b_addr + hdrsize, length);
 		amountdone += length;
 		value += length;
diff --git a/repair/dino_chunks.c b/repair/dino_chunks.c
index 17de95f5..1c7f9c8d 100644
--- a/repair/dino_chunks.c
+++ b/repair/dino_chunks.c
@@ -689,7 +689,7 @@ process_inode_chunk(
 
 		pftrace("readbuf %p (%llu, %d) in AG %d", bplist[bp_index],
 			(long long)XFS_BUF_ADDR(bplist[bp_index]),
-			XFS_BUF_COUNT(bplist[bp_index]), agno);
+			bplist[bp_index]->b_bcount, agno);
 
 		bplist[bp_index]->b_ops = &xfs_inode_buf_ops;
 
diff --git a/repair/prefetch.c b/repair/prefetch.c
index 0783d225..ff40d18d 100644
--- a/repair/prefetch.c
+++ b/repair/prefetch.c
@@ -403,7 +403,7 @@ pf_read_inode_dirs(
 	if (bp->b_error)
 		return;
 
-	for (icnt = 0; icnt < (XFS_BUF_COUNT(bp) >> mp->m_sb.sb_inodelog); icnt++) {
+	for (icnt = 0; icnt < (bp->b_bcount >> mp->m_sb.sb_inodelog); icnt++) {
 		dino = xfs_make_iptr(mp, bp, icnt);
 
 		/*
-- 
2.14.3

[PATCH 4/4] Get rid of XFS_BUF_SET_COUNT() macro

[Carlos Maiolino]

Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com>
---
V2:
	Fix indentation

 libxfs/libxfs_io.h        | 3 +--
 libxlog/xfs_log_recover.c | 2 +-
 2 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/libxfs/libxfs_io.h b/libxfs/libxfs_io.h
index f429d667..78c29445 100644
--- a/libxfs/libxfs_io.h
+++ b/libxfs/libxfs_io.h
@@ -102,11 +102,10 @@ enum xfs_buf_flags_t {	/* b_flags bits */
 #define XFS_BUF_SIZE(bp)		((bp)->b_bcount)
 #define XFS_BUF_SET_PTR(bp,p,cnt)	({	\
 	(bp)->b_addr = (char *)(p);		\
-	XFS_BUF_SET_COUNT(bp,cnt);		\
+	(bp)->b_bcount = (cnt);			\
 })
 
 #define XFS_BUF_SET_ADDR(bp,blk)	((bp)->b_bn = (blk))
-#define XFS_BUF_SET_COUNT(bp,cnt)	((bp)->b_bcount = (cnt))
 
 #define XFS_BUF_SET_PRIORITY(bp,pri)	cache_node_set_priority( \
 						libxfs_bcache, \
diff --git a/libxlog/xfs_log_recover.c b/libxlog/xfs_log_recover.c
index 6bd000c0..68989930 100644
--- a/libxlog/xfs_log_recover.c
+++ b/libxlog/xfs_log_recover.c
@@ -131,7 +131,7 @@ xlog_bread_noalign(
 	ASSERT(BBTOB(nbblks) <= XFS_BUF_SIZE(bp));
 
 	XFS_BUF_SET_ADDR(bp, log->l_logBBstart + blk_no);
-	XFS_BUF_SET_COUNT(bp, BBTOB(nbblks));
+	bp->b_bcount = BBTOB(nbblks);
 	bp->b_error = 0;
 
 	return libxfs_readbufr(log->l_dev, XFS_BUF_ADDR(bp), bp, nbblks, 0);
-- 
2.14.3

Re: [PATCH 0/4 V2] Remove a few macros

[Eric Sandeen]

On 3/7/18 3:05 AM, Carlos Maiolino wrote:
> Hi,
> 
> this is a second version of the patchset aimed to clean up some old macros from
> xfsprogs, as the objective to bring xfs_buf handling code closer to kernel code.
> 
> It fixes some pointer castings Dave mentioned on the V1 patch, also, it removes
> a few unneeded castings I spotted while reviewing the ones Dave mentioned.
> 
> Cheers
> 
> Carlos Maiolino (4):
>    Get rid of XFS_BUF_PTR() macro
>    Get rid of XFS_BUF_TARGET() macro
>    get rid of XFS_BUF_COUNT() macro
>    Get rid of XFS_BUF_SET_COUNT() macro

Sorry, I'd still like to merge these but they need a rebase now.
And did the casting-wars between dchinner & hch ever get resolved?

Thanks,
-Eric

Re: [PATCH 0/4 V2] Remove a few macros

[Carlos Maiolino]

On Thu, May 31, 2018 at 11:58:51AM -0500, Eric Sandeen wrote:
> On 3/7/18 3:05 AM, Carlos Maiolino wrote:
> > Hi,
> > 
> > this is a second version of the patchset aimed to clean up some old macros from
> > xfsprogs, as the objective to bring xfs_buf handling code closer to kernel code.
> > 
> > It fixes some pointer castings Dave mentioned on the V1 patch, also, it removes
> > a few unneeded castings I spotted while reviewing the ones Dave mentioned.
> > 
> > Cheers
> > 
> > Carlos Maiolino (4):
> >    Get rid of XFS_BUF_PTR() macro
> >    Get rid of XFS_BUF_TARGET() macro
> >    get rid of XFS_BUF_COUNT() macro
> >    Get rid of XFS_BUF_SET_COUNT() macro
> 
> Sorry, I'd still like to merge these but they need a rebase now.
> And did the casting-wars between dchinner & hch ever get resolved?
> 


I don't know actually, I don't remember seeing Dave replying agreeing or not
with hch

> Thanks,
> -Eric

-- 
Carlos

Re: [PATCH 0/4 V2] Remove a few macros

[Dave Chinner]

On Thu, May 31, 2018 at 07:19:09PM +0200, Carlos Maiolino wrote:
> On Thu, May 31, 2018 at 11:58:51AM -0500, Eric Sandeen wrote:
> > On 3/7/18 3:05 AM, Carlos Maiolino wrote:
> > > Hi,
> > > 
> > > this is a second version of the patchset aimed to clean up some old macros from
> > > xfsprogs, as the objective to bring xfs_buf handling code closer to kernel code.
> > > 
> > > It fixes some pointer castings Dave mentioned on the V1 patch, also, it removes
> > > a few unneeded castings I spotted while reviewing the ones Dave mentioned.
> > > 
> > > Cheers
> > > 
> > > Carlos Maiolino (4):
> > >    Get rid of XFS_BUF_PTR() macro
> > >    Get rid of XFS_BUF_TARGET() macro
> > >    get rid of XFS_BUF_COUNT() macro
> > >    Get rid of XFS_BUF_SET_COUNT() macro
> > 
> > Sorry, I'd still like to merge these but they need a rebase now.
> > And did the casting-wars between dchinner & hch ever get resolved?
> > 
> 
> 
> I don't know actually, I don't remember seeing Dave replying agreeing or not
> with hch

What was there to argue? Christoph wants us to rely on undocumented,
compiler specific behaviour(*), I want it the pointer arithmetic to
be explicitly correct with a cast.

Maintainer's choice, really.

Cheers,

Dave.

(*) It's been repeatedly demonstrated that the gcc developers don't
care if they break code that relies on undefined behaviour in the C
standard.
-- 
Dave Chinner
david@fromorbit.com

Re: [PATCH 0/4 V2] Remove a few macros

[Darrick J. Wong]

On Fri, Jun 01, 2018 at 09:11:53AM +1000, Dave Chinner wrote:
> On Thu, May 31, 2018 at 07:19:09PM +0200, Carlos Maiolino wrote:
> > On Thu, May 31, 2018 at 11:58:51AM -0500, Eric Sandeen wrote:
> > > On 3/7/18 3:05 AM, Carlos Maiolino wrote:
> > > > Hi,
> > > > 
> > > > this is a second version of the patchset aimed to clean up some old macros from
> > > > xfsprogs, as the objective to bring xfs_buf handling code closer to kernel code.
> > > > 
> > > > It fixes some pointer castings Dave mentioned on the V1 patch, also, it removes
> > > > a few unneeded castings I spotted while reviewing the ones Dave mentioned.
> > > > 
> > > > Cheers
> > > > 
> > > > Carlos Maiolino (4):
> > > >    Get rid of XFS_BUF_PTR() macro
> > > >    Get rid of XFS_BUF_TARGET() macro
> > > >    get rid of XFS_BUF_COUNT() macro
> > > >    Get rid of XFS_BUF_SET_COUNT() macro
> > > 
> > > Sorry, I'd still like to merge these but they need a rebase now.
> > > And did the casting-wars between dchinner & hch ever get resolved?
> > > 
> > 
> > 
> > I don't know actually, I don't remember seeing Dave replying agreeing or not
> > with hch
> 
> What was there to argue? Christoph wants us to rely on undocumented,
> compiler specific behaviour(*), I want it the pointer arithmetic to
> be explicitly correct with a cast.
> 
> Maintainer's choice, really.

Explicit (char *) cast so it works correctly, please. :)

--D

> Cheers,
> 
> Dave.
> 
> (*) It's been repeatedly demonstrated that the gcc developers don't
> care if they break code that relies on undefined behaviour in the C
> standard.
> -- 
> Dave Chinner
> david@fromorbit.com
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 0/4 V2] Remove a few macros

[Christoph Hellwig]

On Fri, Jun 01, 2018 at 09:11:53AM +1000, Dave Chinner wrote:
> 
> What was there to argue? Christoph wants us to rely on undocumented,
> compiler specific behaviour(*),

It is not undocumented.  Sections like this have been part of the
gcc manual for at least 25 years:

https://gcc.gnu.org/onlinedocs/gcc/Pointer-Arith.html

> I want it the pointer arithmetic to
> be explicitly correct with a cast.
> 
> Maintainer's choice, really.
> 
> Cheers,
> 
> Dave.
> 
> (*) It's been repeatedly demonstrated that the gcc developers don't
> care if they break code that relies on undefined behaviour in the C
> standard.

But this behavior is not undefined.  It is explicitly defined for gcc,
with other compilers (LLVM, icc) following that specification.

Re: [PATCH 0/4 V2] Remove a few macros

[Eric Sandeen]

On 3/7/18 1:05 AM, Carlos Maiolino wrote:
> Hi,
> 
> this is a second version of the patchset aimed to clean up some old macros from
> xfsprogs, as the objective to bring xfs_buf handling code closer to kernel code.
> 
> It fixes some pointer castings Dave mentioned on the V1 patch, also, it removes
> a few unneeded castings I spotted while reviewing the ones Dave mentioned.
> 
> Cheers

Ok, I'll take this, with the explicit char * casts (seems to be the popular
vote, and TBH I guess I prefer the explicit nature of that approach).

There are a couple of extra ones now in your first patch though,
i.e.

ptr = (char *)bp->b_addr;

is not needed.

I fixed up minor conflicts, no need to resend.

-Eric
 
> Carlos Maiolino (4):
>   Get rid of XFS_BUF_PTR() macro
>   Get rid of XFS_BUF_TARGET() macro
>   get rid of XFS_BUF_COUNT() macro
>   Get rid of XFS_BUF_SET_COUNT() macro
> 
>  libxfs/libxfs_io.h        |  6 +-----
>  libxfs/logitem.c          |  2 +-
>  libxfs/rdwr.c             | 10 +++++-----
>  libxfs/trans.c            |  2 +-
>  libxlog/xfs_log_recover.c |  2 +-
>  logprint/log_print_all.c  |  4 ++--
>  mkfs/proto.c              |  8 ++++----
>  mkfs/xfs_mkfs.c           | 14 +++++++-------
>  repair/agheader.c         |  8 ++++----
>  repair/attr_repair.c      |  4 ++--
>  repair/dino_chunks.c      |  2 +-
>  repair/phase6.c           |  4 ++--
>  repair/prefetch.c         |  4 ++--
>  13 files changed, 33 insertions(+), 37 deletions(-)
>