linux-xfs.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Darrick J. Wong" <darrick.wong@oracle.com>
To: sandeen@sandeen.net, darrick.wong@oracle.com
Cc: linux-xfs@vger.kernel.org
Subject: [PATCH 12/18] xfs_scrub: remove moveon from phase 5 functions
Date: Tue, 22 Oct 2019 11:51:44 -0700	[thread overview]
Message-ID: <157177030411.1461658.6044923482666014455.stgit@magnolia> (raw)
In-Reply-To: <157177022106.1461658.18024534947316119946.stgit@magnolia>

From: Darrick J. Wong <darrick.wong@oracle.com>

Replace the moveon returns in the phase 5 code with a direct integer
error return.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 scrub/phase5.c |  184 ++++++++++++++++++++++++++++++--------------------------
 1 file changed, 97 insertions(+), 87 deletions(-)


diff --git a/scrub/phase5.c b/scrub/phase5.c
index c2cccd76..2641a7fb 100644
--- a/scrub/phase5.c
+++ b/scrub/phase5.c
@@ -31,9 +31,11 @@
  * terminal control characters and escape sequences, since that could be used
  * to do something naughty to the user's computer and/or break scripts.  XFS
  * doesn't consider any byte sequence invalid, so don't flag these as errors.
+ *
+ * Returns 0 for success or -1 for error.  This function logs errors.
  */
-static bool
-xfs_scrub_check_name(
+static int
+simple_check_name(
 	struct scrub_ctx	*ctx,
 	struct descr		*dsc,
 	const char		*namedescr,
@@ -46,7 +48,7 @@ xfs_scrub_check_name(
 	/* Complain about zero length names. */
 	if (*name == '\0' && should_warn_about_name(ctx)) {
 		str_warn(ctx, descr_render(dsc), _("Zero length name found."));
-		return true;
+		return 0;
 	}
 
 	/* control characters */
@@ -61,7 +63,7 @@ xfs_scrub_check_name(
 		errname = string_escape(name);
 		if (!errname) {
 			str_errno(ctx, descr_render(dsc));
-			return false;
+			return -1;
 		}
 		str_info(ctx, descr_render(dsc),
 _("Control character found in %s name \"%s\"."),
@@ -69,15 +71,15 @@ _("Control character found in %s name \"%s\"."),
 		free(errname);
 	}
 
-	return true;
+	return 0;
 }
 
 /*
  * Iterate a directory looking for filenames with problematic
  * characters.
  */
-static bool
-xfs_scrub_scan_dirents(
+static int
+check_dirent_names(
 	struct scrub_ctx	*ctx,
 	struct descr		*dsc,
 	int			*fd,
@@ -86,45 +88,45 @@ xfs_scrub_scan_dirents(
 	struct unicrash		*uc = NULL;
 	DIR			*dir;
 	struct dirent		*dentry;
-	bool			moveon = true;
 	int			ret;
 
 	dir = fdopendir(*fd);
 	if (!dir) {
 		str_errno(ctx, descr_render(dsc));
-		moveon = false;
-		goto out;
+		return errno;
 	}
 	*fd = -1; /* closedir will close *fd for us */
 
 	ret = unicrash_dir_init(&uc, ctx, bstat);
 	if (ret) {
 		str_liberror(ctx, ret, descr_render(dsc));
-		moveon = false;
 		goto out_unicrash;
 	}
 
+	errno = 0;
 	dentry = readdir(dir);
 	while (dentry) {
-		if (uc) {
+		if (uc)
 			ret = unicrash_check_dir_name(uc, dsc, dentry);
-			if (ret) {
-				str_liberror(ctx, ret, descr_render(dsc));
-				moveon = false;
-			}
-		} else
-			moveon = xfs_scrub_check_name(ctx, dsc,
-					_("directory"), dentry->d_name);
-		if (!moveon)
+		else
+			ret = simple_check_name(ctx, dsc, _("directory"),
+					dentry->d_name);
+		if (ret) {
+			str_liberror(ctx, ret, descr_render(dsc));
 			break;
+		}
+		errno = 0;
 		dentry = readdir(dir);
 	}
+	if (errno) {
+		ret = errno;
+		str_liberror(ctx, ret, descr_render(dsc));
+	}
 	unicrash_free(uc);
 
 out_unicrash:
 	closedir(dir);
-out:
-	return moveon;
+	return ret;
 }
 
 #ifdef HAVE_LIBATTR
@@ -145,8 +147,8 @@ static const struct attrns_decode attr_ns[] = {
  * Check all the xattr names in a particular namespace of a file handle
  * for Unicode normalization problems or collisions.
  */
-static bool
-xfs_scrub_scan_fhandle_namespace_xattrs(
+static int
+check_xattr_ns_names(
 	struct scrub_ctx		*ctx,
 	struct descr			*dsc,
 	struct xfs_handle		*handle,
@@ -159,14 +161,13 @@ xfs_scrub_scan_fhandle_namespace_xattrs(
 	struct attrlist			*attrlist = (struct attrlist *)attrbuf;
 	struct attrlist_ent		*ent;
 	struct unicrash			*uc = NULL;
-	bool				moveon = true;
 	int				i;
 	int				error;
 
 	error = unicrash_xattr_init(&uc, ctx, bstat);
 	if (error) {
 		str_liberror(ctx, error, descr_render(dsc));
-		return false;
+		return error;
 	}
 
 	memset(attrbuf, 0, XFS_XATTR_LIST_MAX);
@@ -180,20 +181,17 @@ xfs_scrub_scan_fhandle_namespace_xattrs(
 			ent = ATTR_ENTRY(attrlist, i);
 			snprintf(keybuf, XATTR_NAME_MAX, "%s.%s", attr_ns->name,
 					ent->a_name);
-			if (uc) {
+			if (uc)
 				error = unicrash_check_xattr_name(uc, dsc,
 						keybuf);
-				if (error) {
-					str_liberror(ctx, error,
-							descr_render(dsc));
-					moveon = false;
-				}
-			} else
-				moveon = xfs_scrub_check_name(ctx, dsc,
+			else
+				error = simple_check_name(ctx, dsc,
 						_("extended attribute"),
 						keybuf);
-			if (!moveon)
+			if (error) {
+				str_liberror(ctx, error, descr_render(dsc));
 				goto out;
+			}
 		}
 
 		if (!attrlist->al_more)
@@ -201,37 +199,40 @@ xfs_scrub_scan_fhandle_namespace_xattrs(
 		error = attr_list_by_handle(handle, sizeof(*handle), attrbuf,
 				XFS_XATTR_LIST_MAX, attr_ns->flags, &cur);
 	}
-	if (error && errno != ESTALE)
-		str_errno(ctx, descr_render(dsc));
+	if (error) {
+		if (errno == ESTALE)
+			errno = 0;
+		if (errno)
+			str_errno(ctx, descr_render(dsc));
+	}
 out:
 	unicrash_free(uc);
-	return moveon;
+	return error;
 }
 
 /*
  * Check all the xattr names in all the xattr namespaces for problematic
  * characters.
  */
-static bool
-xfs_scrub_scan_fhandle_xattrs(
+static int
+check_xattr_names(
 	struct scrub_ctx		*ctx,
 	struct descr			*dsc,
 	struct xfs_handle		*handle,
 	struct xfs_bulkstat		*bstat)
 {
 	const struct attrns_decode	*ns;
-	bool				moveon = true;
+	int				ret;
 
 	for (ns = attr_ns; ns->name; ns++) {
-		moveon = xfs_scrub_scan_fhandle_namespace_xattrs(ctx, dsc,
-				handle, bstat, ns);
-		if (!moveon)
+		ret = check_xattr_ns_names(ctx, dsc, handle, bstat, ns);
+		if (ret)
 			break;
 	}
-	return moveon;
+	return ret;
 }
 #else
-# define xfs_scrub_scan_fhandle_xattrs(c, d, h, b)	(true)
+# define check_xattr_names(c, d, h, b)	(0)
 #endif /* HAVE_LIBATTR */
 
 static int
@@ -255,26 +256,25 @@ render_ino_from_handle(
  * Check for potential Unicode collisions in names.
  */
 static int
-xfs_scrub_connections(
+check_inode_names(
 	struct scrub_ctx	*ctx,
 	struct xfs_handle	*handle,
 	struct xfs_bulkstat	*bstat,
 	void			*arg)
 {
-	bool			*pmoveon = arg;
 	DEFINE_DESCR(dsc, ctx, render_ino_from_handle);
-	bool			moveon = true;
+	bool			*aborted = arg;
 	int			fd = -1;
-	int			error;
+	int			error = 0;
+	int			err2;
 
 	descr_set(&dsc, bstat);
 	background_sleep();
 
 	/* Warn about naming problems in xattrs. */
 	if (bstat->bs_xflags & FS_XFLAG_HASATTR) {
-		moveon = xfs_scrub_scan_fhandle_xattrs(ctx, &dsc, handle,
-				bstat);
-		if (!moveon)
+		error = check_xattr_names(ctx, &dsc, handle, bstat);
+		if (error)
 			goto out;
 	}
 
@@ -282,7 +282,8 @@ xfs_scrub_connections(
 	if (S_ISDIR(bstat->bs_mode)) {
 		fd = scrub_open_handle(handle);
 		if (fd < 0) {
-			if (errno == ESTALE)
+			error = errno;
+			if (error == ESTALE)
 				return ESTALE;
 			str_errno(ctx, descr_render(&dsc));
 			goto out;
@@ -291,21 +292,27 @@ xfs_scrub_connections(
 
 	/* Warn about naming problems in the directory entries. */
 	if (fd >= 0 && S_ISDIR(bstat->bs_mode)) {
-		moveon = xfs_scrub_scan_dirents(ctx, &dsc, &fd, bstat);
-		if (!moveon)
+		error = check_dirent_names(ctx, &dsc, &fd, bstat);
+		if (error)
 			goto out;
 	}
 
 out:
 	progress_add(1);
 	if (fd >= 0) {
-		error = close(fd);
-		if (error)
+		err2 = close(fd);
+		if (err2)
 			str_errno(ctx, descr_render(&dsc));
+		if (!error && err2)
+			error = err2;
 	}
-	if (!moveon)
-		*pmoveon = false;
-	return *pmoveon ? 0 : XFS_ITERATE_INODES_ABORT;
+
+	if (error)
+		*aborted = true;
+	if (!error && *aborted)
+		error = ECANCELED;
+
+	return error;
 }
 
 #ifndef FS_IOC_GETFSLABEL
@@ -327,20 +334,19 @@ scrub_render_mountpoint(
  * Check the filesystem label for Unicode normalization problems or misleading
  * sequences.
  */
-static bool
-xfs_scrub_fs_label(
+static int
+check_fs_label(
 	struct scrub_ctx		*ctx)
 {
 	DEFINE_DESCR(dsc, ctx, scrub_render_mountpoint);
 	char				label[FSLABEL_MAX];
 	struct unicrash			*uc = NULL;
-	bool				moveon = true;
 	int				error;
 
 	error = unicrash_fs_label_init(&uc, ctx);
 	if (error) {
 		str_liberror(ctx, error, descr_render(&dsc));
-		return false;
+		return error;
 	}
 
 	descr_set(&dsc, NULL);
@@ -349,7 +355,7 @@ xfs_scrub_fs_label(
 	error = ioctl(ctx->mnt.fd, FS_IOC_GETFSLABEL, &label);
 	if (error) {
 		if (errno != EOPNOTSUPP && errno != ENOTTY) {
-			moveon = false;
+			error = errno;
 			perror(ctx->mntpoint);
 		}
 		goto out;
@@ -360,45 +366,49 @@ xfs_scrub_fs_label(
 		goto out;
 
 	/* Otherwise check for weirdness. */
-	if (uc) {
+	if (uc)
 		error = unicrash_check_fs_label(uc, &dsc, label);
-		if (error) {
-			str_liberror(ctx, error, descr_render(&dsc));
-			moveon = false;
-		}
-	} else
-		moveon = xfs_scrub_check_name(ctx, &dsc, _("filesystem label"),
+	else
+		error = simple_check_name(ctx, &dsc, _("filesystem label"),
 				label);
-	if (!moveon)
-		goto out;
+	if (error)
+		str_liberror(ctx, error, descr_render(&dsc));
 out:
 	unicrash_free(uc);
-	return moveon;
+	return error;
 }
 
 /* Check directory connectivity. */
-bool
-xfs_scan_connections(
+int
+phase5_func(
 	struct scrub_ctx	*ctx)
 {
-	bool			moveon = true;
+	bool			aborted = false;
 	int			ret;
 
 	if (ctx->corruptions_found || ctx->unfixable_errors) {
 		str_info(ctx, ctx->mntpoint,
 _("Filesystem has errors, skipping connectivity checks."));
-		return true;
+		return 0;
 	}
 
-	ret = xfs_scrub_fs_label(ctx);
+	ret = check_fs_label(ctx);
 	if (ret)
-		return false;
+		return ret;
 
-	ret = scrub_scan_all_inodes(ctx, xfs_scrub_connections, &moveon);
+	ret = scrub_scan_all_inodes(ctx, check_inode_names, &aborted);
 	if (ret)
-		return false;
-	if (!moveon)
-		return false;
+		return ret;
+	if (aborted)
+		return ECANCELED;
+
 	xfs_scrub_report_preen_triggers(ctx);
-	return true;
+	return 0;
+}
+
+bool
+xfs_scan_connections(
+	struct scrub_ctx	*ctx)
+{
+	return phase5_func(ctx) == 0;
 }


  parent reply	other threads:[~2019-10-22 18:51 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-10-22 18:50 [PATCH 00/18] xfs_scrub: remove moveon space aliens Darrick J. Wong
2019-10-22 18:50 ` [PATCH 01/18] xfs_scrub: remove moveon from filemap iteration Darrick J. Wong
2019-11-06 21:01   ` [PATCH v2 " Darrick J. Wong
2019-11-06 21:37     ` Eric Sandeen
2019-10-22 18:50 ` [PATCH 02/18] xfs_scrub: remove moveon from the fscounters functions Darrick J. Wong
2019-10-22 18:50 ` [PATCH 03/18] xfs_scrub: remove moveon from inode iteration Darrick J. Wong
2019-10-22 18:50 ` [PATCH 04/18] xfs_scrub: remove moveon from vfs directory tree iteration Darrick J. Wong
2019-10-22 18:50 ` [PATCH 05/18] xfs_scrub: remove moveon from spacemap Darrick J. Wong
2019-11-06 21:02   ` [PATCH v2 " Darrick J. Wong
2019-10-22 18:51 ` [PATCH 06/18] xfs_scrub: remove moveon from unicode name collision helpers Darrick J. Wong
2019-10-22 18:51 ` [PATCH 07/18] xfs_scrub: remove moveon from progress report helpers Darrick J. Wong
2019-10-22 18:51 ` [PATCH 08/18] xfs_scrub: remove moveon from scrub ioctl wrappers Darrick J. Wong
2019-10-22 18:51 ` [PATCH 09/18] xfs_scrub: remove moveon from repair action list helpers Darrick J. Wong
2019-10-22 18:51 ` [PATCH 10/18] xfs_scrub: remove moveon from phase 7 functions Darrick J. Wong
2019-10-22 18:51 ` [PATCH 11/18] xfs_scrub: remove moveon from phase 6 functions Darrick J. Wong
2019-11-06 21:02   ` [PATCH v2 " Darrick J. Wong
2019-10-22 18:51 ` Darrick J. Wong [this message]
2019-10-22 18:51 ` [PATCH 13/18] xfs_scrub: remove moveon from phase 4 functions Darrick J. Wong
2019-10-22 18:51 ` [PATCH 14/18] xfs_scrub: remove moveon from phase 3 functions Darrick J. Wong
2019-10-22 18:52 ` [PATCH 15/18] xfs_scrub: remove moveon from phase 2 functions Darrick J. Wong
2019-10-22 18:52 ` [PATCH 16/18] xfs_scrub: remove moveon from phase 1 functions Darrick J. Wong
2019-10-22 18:52 ` [PATCH 17/18] xfs_scrub: remove XFS_ITERATE_INODES_ABORT from inode iterator Darrick J. Wong
2019-10-22 18:52 ` [PATCH 18/18] xfs_scrub: remove moveon from main program Darrick J. Wong
2019-11-06 21:03   ` [PATCH v2 " Darrick J. Wong
2019-11-06 22:24 ` [PATCH 00/18] xfs_scrub: remove moveon space aliens Eric Sandeen
  -- strict thread matches above, loose matches on Subject: below --
2019-09-25 21:38 Darrick J. Wong
2019-09-25 21:39 ` [PATCH 12/18] xfs_scrub: remove moveon from phase 5 functions Darrick J. Wong
2019-09-06  3:40 [PATCH 00/18] xfs_scrub: remove moveon space aliens Darrick J. Wong
2019-09-06  3:42 ` [PATCH 12/18] xfs_scrub: remove moveon from phase 5 functions Darrick J. Wong

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=157177030411.1461658.6044923482666014455.stgit@magnolia \
    --to=darrick.wong@oracle.com \
    --cc=linux-xfs@vger.kernel.org \
    --cc=sandeen@sandeen.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).