diff mbox series

[01/18] libxfs: clean up readbuf flags

Message ID 158216296035.602314.7876331402312462299.stgit@magnolia (mailing list archive)
State Deferred, archived
Headers show
Series xfsprogs: refactor buffer function names | expand

Commit Message

Darrick J. Wong Feb. 20, 2020, 1:42 a.m. UTC 
From: Darrick J. Wong <darrick.wong@oracle.com>

Create a separate namespace for libxfs_readbuf() flags so that it's a
little more obvious when we're trying to use the "read or die" logic.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 libxfs/init.c      |   11 ++++++++---
 libxfs/libxfs_io.h |    3 +++
 libxfs/rdwr.c      |    4 ++--
 mkfs/xfs_mkfs.c    |    4 ++--
 4 files changed, 15 insertions(+), 7 deletions(-)

Comments

Christoph Hellwig Feb. 21, 2020, 2:42 p.m. UTC | #1 
On Wed, Feb 19, 2020 at 05:42:40PM -0800, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> Create a separate namespace for libxfs_readbuf() flags so that it's a
> little more obvious when we're trying to use the "read or die" logic.

Can we just kill this damn flag instead?  Life would be much simpler
if the exit simply moved to the caller.  It also kills the exit call
in a library anti-pattern (although of course due to being conditional
it isn't as bad as the real antipattern from the X11 libraries..)
Darrick J. Wong Feb. 21, 2020, 3:55 p.m. UTC | #2 
On Fri, Feb 21, 2020 at 06:42:47AM -0800, Christoph Hellwig wrote:
> On Wed, Feb 19, 2020 at 05:42:40PM -0800, Darrick J. Wong wrote:
> > From: Darrick J. Wong <darrick.wong@oracle.com>
> > 
> > Create a separate namespace for libxfs_readbuf() flags so that it's a
> > little more obvious when we're trying to use the "read or die" logic.
> 
> Can we just kill this damn flag instead?  Life would be much simpler
> if the exit simply moved to the caller.  It also kills the exit call
> in a library anti-pattern (although of course due to being conditional
> it isn't as bad as the real antipattern from the X11 libraries..)

Heh.  It was only now that I realized that there are ~8 callers of the
"fail on ioerror" flag.  Yes, let's get rid of them both.

--D
diff mbox series

Patch

diff --git a/libxfs/init.c b/libxfs/init.c
index d1d3f4df..428497f0 100644
--- a/libxfs/init.c
+++ b/libxfs/init.c
@@ -640,6 +640,7 @@  libxfs_mount(
 	xfs_buf_t	*bp;
 	xfs_sb_t	*sbp;
 	int		error;
+	int		readflags = 0;
 
 	libxfs_buftarg_init(mp, dev, logdev, rtdev);
 
@@ -716,9 +717,13 @@  libxfs_mount(
 	if (dev == 0)	/* maxtrres, we have no device so leave now */
 		return mp;
 
+	/* device size checks must pass unless we're a debugger. */
+	if (!(flags & LIBXFS_MOUNT_DEBUGGER))
+		readflags |= LIBXFS_READBUF_FAIL_EXIT;
+
 	bp = libxfs_readbuf(mp->m_dev,
 			d - XFS_FSS_TO_BB(mp, 1), XFS_FSS_TO_BB(mp, 1),
-			!(flags & LIBXFS_MOUNT_DEBUGGER), NULL);
+			readflags, NULL);
 	if (!bp) {
 		fprintf(stderr, _("%s: data size check failed\n"), progname);
 		if (!(flags & LIBXFS_MOUNT_DEBUGGER))
@@ -733,7 +738,7 @@  libxfs_mount(
 		     (!(bp = libxfs_readbuf(mp->m_logdev_targp,
 					d - XFS_FSB_TO_BB(mp, 1),
 					XFS_FSB_TO_BB(mp, 1),
-					!(flags & LIBXFS_MOUNT_DEBUGGER), NULL))) ) {
+					readflags, NULL))) ) {
 			fprintf(stderr, _("%s: log size checks failed\n"),
 					progname);
 			if (!(flags & LIBXFS_MOUNT_DEBUGGER))
@@ -760,7 +765,7 @@  libxfs_mount(
 	if (sbp->sb_agcount > 1000000) {
 		bp = libxfs_readbuf(mp->m_dev,
 				XFS_AG_DADDR(mp, sbp->sb_agcount - 1, 0), 1,
-				!(flags & LIBXFS_MOUNT_DEBUGGER), NULL);
+				readflags, NULL);
 		if (bp->b_error) {
 			fprintf(stderr, _("%s: read of AG %u failed\n"),
 						progname, sbp->sb_agcount);
diff --git a/libxfs/libxfs_io.h b/libxfs/libxfs_io.h
index fc0fd060..b294e659 100644
--- a/libxfs/libxfs_io.h
+++ b/libxfs/libxfs_io.h
@@ -124,6 +124,9 @@  extern struct cache_operations	libxfs_bcache_operations;
 
 #define LIBXFS_GETBUF_TRYLOCK	(1 << 0)
 
+/* Exit on buffer read error */
+#define LIBXFS_READBUF_FAIL_EXIT	(1 << 0)
+
 #ifdef XFS_BUF_TRACING
 
 #define libxfs_readbuf(dev, daddr, len, flags, ops) \
diff --git a/libxfs/rdwr.c b/libxfs/rdwr.c
index 92e497f9..32619a8d 100644
--- a/libxfs/rdwr.c
+++ b/libxfs/rdwr.c
@@ -911,13 +911,13 @@  __read_buf(int fd, void *buf, int len, off64_t offset, int flags)
 		int error = errno;
 		fprintf(stderr, _("%s: read failed: %s\n"),
 			progname, strerror(error));
-		if (flags & LIBXFS_EXIT_ON_FAILURE)
+		if (flags & LIBXFS_READBUF_FAIL_EXIT)
 			exit(1);
 		return -error;
 	} else if (sts != len) {
 		fprintf(stderr, _("%s: error - read only %d of %d bytes\n"),
 			progname, sts, len);
-		if (flags & LIBXFS_EXIT_ON_FAILURE)
+		if (flags & LIBXFS_READBUF_FAIL_EXIT)
 			exit(1);
 		return -EIO;
 	}
diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c
index 6b182264..a57046f1 100644
--- a/mkfs/xfs_mkfs.c
+++ b/mkfs/xfs_mkfs.c
@@ -3577,7 +3577,7 @@  rewrite_secondary_superblocks(
 			XFS_AGB_TO_DADDR(mp, mp->m_sb.sb_agcount - 1,
 				XFS_SB_DADDR),
 			XFS_FSS_TO_BB(mp, 1),
-			LIBXFS_EXIT_ON_FAILURE, &xfs_sb_buf_ops);
+			LIBXFS_READBUF_FAIL_EXIT, &xfs_sb_buf_ops);
 	XFS_BUF_TO_SBP(buf)->sb_rootino = cpu_to_be64(mp->m_sb.sb_rootino);
 	libxfs_writebuf(buf, LIBXFS_EXIT_ON_FAILURE);
 
@@ -3589,7 +3589,7 @@  rewrite_secondary_superblocks(
 			XFS_AGB_TO_DADDR(mp, (mp->m_sb.sb_agcount - 1) / 2,
 				XFS_SB_DADDR),
 			XFS_FSS_TO_BB(mp, 1),
-			LIBXFS_EXIT_ON_FAILURE, &xfs_sb_buf_ops);
+			LIBXFS_READBUF_FAIL_EXIT, &xfs_sb_buf_ops);
 	XFS_BUF_TO_SBP(buf)->sb_rootino = cpu_to_be64(mp->m_sb.sb_rootino);
 	libxfs_writebuf(buf, LIBXFS_EXIT_ON_FAILURE);
 }