All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] xfs_grow: Remove xflag and iflag to reduce redundant temporary variables.
@ 2023-12-07  8:39 ` wuyifeng (C)
  2023-12-08  8:28   ` wuyifeng (C)
  2023-12-11 11:10   ` Carlos Maiolino
  0 siblings, 2 replies; 6+ messages in thread
From: wuyifeng (C) @ 2023-12-07  8:39 UTC (permalink / raw)
  To: cem, Darrick J. Wong, linux-xfs; +Cc: louhongxiang

[-- Attachment #1: Type: text/plain, Size: 4130 bytes --]

I found that iflag and xflag can be combined with lflag to reduce the 
number of redundant local variables, which is a refactoring to improve 
code readability.Signed-off-by: Wu YiFeng <wuyifeng10@huawei.com>

Please help me review, thanks.

---
  growfs/xfs_growfs.c | 22 +++++++++++-----------
  1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/growfs/xfs_growfs.c b/growfs/xfs_growfs.c
index 683961f6..5fb1a9d2 100644
--- a/growfs/xfs_growfs.c
+++ b/growfs/xfs_growfs.c
@@ -8,6 +8,10 @@
  #include "libfrog/paths.h"
  #include "libfrog/fsgeom.h"

+#define LOG_GROW    (1<<0)    /* -l flag: grow log section */
+#define LOG_EXT2INT    (1<<1)    /* -i flag: convert log from external 
to internal format */
+#define LOG_INT2EXT    (1<<2)    /* -x flag: convert log from internal 
to external format */
+
  static void
  usage(void)
  {
@@ -45,7 +49,6 @@ main(int argc, char **argv)
      long            esize;    /* new rt extent size */
      int            ffd;    /* mount point file descriptor */
      struct xfs_fsop_geom    geo;    /* current fs geometry */
-    int            iflag;    /* -i flag */
      int            isint;    /* log is currently internal */
      int            lflag;    /* -l flag */
      long long        lsize;    /* new log size in fs blocks */
@@ -55,7 +58,6 @@ main(int argc, char **argv)
      struct xfs_fsop_geom    ngeo;    /* new fs geometry */
      int            rflag;    /* -r flag */
      long long        rsize;    /* new rt size in fs blocks */
-    int            xflag;    /* -x flag */
      char            *fname;    /* mount point name */
      char            *datadev; /* data device name */
      char            *logdev;  /*  log device name */
@@ -72,7 +74,7 @@ main(int argc, char **argv)

      maxpct = esize = 0;
      dsize = lsize = rsize = 0LL;
-    aflag = dflag = iflag = lflag = mflag = nflag = rflag = xflag = 0;
+    aflag = dflag = lflag = mflag = nflag = rflag = 0;

      while ((c = getopt(argc, argv, "dD:e:ilL:m:np:rR:t:xV")) != EOF) {
          switch (c) {
@@ -87,13 +89,13 @@ main(int argc, char **argv)
              rflag = 1;
              break;
          case 'i':
-            lflag = iflag = 1;
+            lflag |= LOG_EXT2INT;
              break;
          case 'L':
              lsize = strtoll(optarg, NULL, 10);
              fallthrough;
          case 'l':
-            lflag = 1;
+            lflag |= LOG_GROW;
              break;
          case 'm':
              mflag = 1;
@@ -115,7 +117,7 @@ main(int argc, char **argv)
              mtab_file = optarg;
              break;
          case 'x':
-            lflag = xflag = 1;
+            lflag |= LOG_INT2EXT;
              break;
          case 'V':
              printf(_("%s version %s\n"), progname, VERSION);
@@ -124,9 +126,7 @@ main(int argc, char **argv)
              usage();
          }
      }
-    if (argc - optind != 1)
-        usage();
-    if (iflag && xflag)
+    if (argc - optind != 1 || ((lflag & LOG_EXT2INT) && (lflag & 
LOG_INT2EXT)))
          usage();
      if (dflag + lflag + rflag + mflag == 0)
          aflag = 1;
@@ -323,9 +323,9 @@ _("[EXPERIMENTAL] try to shrink unused space %lld, 
old size is %lld\n"),

          if (!lsize)
              lsize = dlsize / (geo.blocksize / BBSIZE);
-        if (iflag)
+        if (lflag & LOG_EXT2INT)
              in.isint = 1;
-        else if (xflag)
+        else if (lflag & LOG_INT2EXT)
              in.isint = 0;
          else
              in.isint = xi.logBBsize == 0;
-- 
2.33.0


[-- Attachment #2: 0001-xfs_grow-Remove-redundant-xflag-and-iflag.patch --]
[-- Type: text/plain, Size: 3135 bytes --]

From 74c9fe3337a302385999b57ceb819b3439cdbd9c Mon Sep 17 00:00:00 2001
From: Wu YiFeng <wuyifeng10@huawei.com>
Date: Thu, 7 Dec 2023 15:47:08 +0800
Subject: [PATCH] xfs_grow: Remove xflag and iflag to reduce redundant
 temporary variables.

Both xflag and iflag are log flags. We can use the bits of lflag to
indicate all log flags, which is a small code reconstruction.

Signed-off-by: Wu YiFeng <wuyifeng10@huawei.com>
---
 growfs/xfs_growfs.c | 22 +++++++++++-----------
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/growfs/xfs_growfs.c b/growfs/xfs_growfs.c
index 683961f6..5fb1a9d2 100644
--- a/growfs/xfs_growfs.c
+++ b/growfs/xfs_growfs.c
@@ -8,6 +8,10 @@
 #include "libfrog/paths.h"
 #include "libfrog/fsgeom.h"
 
+#define LOG_GROW	(1<<0)	/* -l flag: grow log section */
+#define LOG_EXT2INT	(1<<1)	/* -i flag: convert log from external to internal format */
+#define LOG_INT2EXT	(1<<2)	/* -x flag: convert log from internal to external format */
+
 static void
 usage(void)
 {
@@ -45,7 +49,6 @@ main(int argc, char **argv)
 	long			esize;	/* new rt extent size */
 	int			ffd;	/* mount point file descriptor */
 	struct xfs_fsop_geom	geo;	/* current fs geometry */
-	int			iflag;	/* -i flag */
 	int			isint;	/* log is currently internal */
 	int			lflag;	/* -l flag */
 	long long		lsize;	/* new log size in fs blocks */
@@ -55,7 +58,6 @@ main(int argc, char **argv)
 	struct xfs_fsop_geom	ngeo;	/* new fs geometry */
 	int			rflag;	/* -r flag */
 	long long		rsize;	/* new rt size in fs blocks */
-	int			xflag;	/* -x flag */
 	char			*fname;	/* mount point name */
 	char			*datadev; /* data device name */
 	char			*logdev;  /*  log device name */
@@ -72,7 +74,7 @@ main(int argc, char **argv)
 
 	maxpct = esize = 0;
 	dsize = lsize = rsize = 0LL;
-	aflag = dflag = iflag = lflag = mflag = nflag = rflag = xflag = 0;
+	aflag = dflag = lflag = mflag = nflag = rflag = 0;
 
 	while ((c = getopt(argc, argv, "dD:e:ilL:m:np:rR:t:xV")) != EOF) {
 		switch (c) {
@@ -87,13 +89,13 @@ main(int argc, char **argv)
 			rflag = 1;
 			break;
 		case 'i':
-			lflag = iflag = 1;
+			lflag |= LOG_EXT2INT;
 			break;
 		case 'L':
 			lsize = strtoll(optarg, NULL, 10);
 			fallthrough;
 		case 'l':
-			lflag = 1;
+			lflag |= LOG_GROW;
 			break;
 		case 'm':
 			mflag = 1;
@@ -115,7 +117,7 @@ main(int argc, char **argv)
 			mtab_file = optarg;
 			break;
 		case 'x':
-			lflag = xflag = 1;
+			lflag |= LOG_INT2EXT;
 			break;
 		case 'V':
 			printf(_("%s version %s\n"), progname, VERSION);
@@ -124,9 +126,7 @@ main(int argc, char **argv)
 			usage();
 		}
 	}
-	if (argc - optind != 1)
-		usage();
-	if (iflag && xflag)
+	if (argc - optind != 1 || ((lflag & LOG_EXT2INT) && (lflag & LOG_INT2EXT)))
 		usage();
 	if (dflag + lflag + rflag + mflag == 0)
 		aflag = 1;
@@ -323,9 +323,9 @@ _("[EXPERIMENTAL] try to shrink unused space %lld, old size is %lld\n"),
 
 		if (!lsize)
 			lsize = dlsize / (geo.blocksize / BBSIZE);
-		if (iflag)
+		if (lflag & LOG_EXT2INT)
 			in.isint = 1;
-		else if (xflag)
+		else if (lflag & LOG_INT2EXT)
 			in.isint = 0;
 		else
 			in.isint = xi.logBBsize == 0;
-- 
2.33.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] xfs_grow: Remove xflag and iflag to reduce redundant temporary variables.
  2023-12-07  8:39 ` [PATCH] xfs_grow: Remove xflag and iflag to reduce redundant temporary variables wuyifeng (C)
@ 2023-12-08  8:28   ` wuyifeng (C)
  2023-12-11 11:10   ` Carlos Maiolino
  1 sibling, 0 replies; 6+ messages in thread
From: wuyifeng (C) @ 2023-12-08  8:28 UTC (permalink / raw)
  To: cem, Darrick J. Wong, linux-xfs; +Cc: louhongxiang

Both xflag and iflag are log flags. We can use the bits of lflag to
indicate all log flags, which is a small code reconstruction.

Signed-off-by: Wu YiFeng <wuyifeng10@huawei.com>
---
  growfs/xfs_growfs.c | 22 +++++++++++-----------
  1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/growfs/xfs_growfs.c b/growfs/xfs_growfs.c
index 683961f6..5fb1a9d2 100644
--- a/growfs/xfs_growfs.c
+++ b/growfs/xfs_growfs.c
@@ -8,6 +8,10 @@
  #include "libfrog/paths.h"
  #include "libfrog/fsgeom.h"

+#define LOG_GROW	(1<<0)	/* -l flag: grow log section */
+#define LOG_EXT2INT	(1<<1)	/* -i flag: convert log from external to 
internal format */
+#define LOG_INT2EXT	(1<<2)	/* -x flag: convert log from internal to 
external format */
+
  static void
  usage(void)
  {
@@ -45,7 +49,6 @@ main(int argc, char **argv)
  	long			esize;	/* new rt extent size */
  	int			ffd;	/* mount point file descriptor */
  	struct xfs_fsop_geom	geo;	/* current fs geometry */
-	int			iflag;	/* -i flag */
  	int			isint;	/* log is currently internal */
  	int			lflag;	/* -l flag */
  	long long		lsize;	/* new log size in fs blocks */
@@ -55,7 +58,6 @@ main(int argc, char **argv)
  	struct xfs_fsop_geom	ngeo;	/* new fs geometry */
  	int			rflag;	/* -r flag */
  	long long		rsize;	/* new rt size in fs blocks */
-	int			xflag;	/* -x flag */
  	char			*fname;	/* mount point name */
  	char			*datadev; /* data device name */
  	char			*logdev;  /*  log device name */
@@ -72,7 +74,7 @@ main(int argc, char **argv)

  	maxpct = esize = 0;
  	dsize = lsize = rsize = 0LL;
-	aflag = dflag = iflag = lflag = mflag = nflag = rflag = xflag = 0;
+	aflag = dflag = lflag = mflag = nflag = rflag = 0;

  	while ((c = getopt(argc, argv, "dD:e:ilL:m:np:rR:t:xV")) != EOF) {
  		switch (c) {
@@ -87,13 +89,13 @@ main(int argc, char **argv)
  			rflag = 1;
  			break;
  		case 'i':
-			lflag = iflag = 1;
+			lflag |= LOG_EXT2INT;
  			break;
  		case 'L':
  			lsize = strtoll(optarg, NULL, 10);
  			fallthrough;
  		case 'l':
-			lflag = 1;
+			lflag |= LOG_GROW;
  			break;
  		case 'm':
  			mflag = 1;
@@ -115,7 +117,7 @@ main(int argc, char **argv)
  			mtab_file = optarg;
  			break;
  		case 'x':
-			lflag = xflag = 1;
+			lflag |= LOG_INT2EXT;
  			break;
  		case 'V':
  			printf(_("%s version %s\n"), progname, VERSION);
@@ -124,9 +126,7 @@ main(int argc, char **argv)
  			usage();
  		}
  	}
-	if (argc - optind != 1)
-		usage();
-	if (iflag && xflag)
+	if (argc - optind != 1 || ((lflag & LOG_EXT2INT) && (lflag & 
LOG_INT2EXT)))
  		usage();
  	if (dflag + lflag + rflag + mflag == 0)
  		aflag = 1;
@@ -323,9 +323,9 @@ _("[EXPERIMENTAL] try to shrink unused space %lld, 
old size is %lld\n"),

  		if (!lsize)
  			lsize = dlsize / (geo.blocksize / BBSIZE);
-		if (iflag)
+		if (lflag & LOG_EXT2INT)
  			in.isint = 1;
-		else if (xflag)
+		else if (lflag & LOG_INT2EXT)
  			in.isint = 0;
  		else
  			in.isint = xi.logBBsize == 0;
-- 
2.33.0

在 2023/12/7 16:39, wuyifeng (C) 写道:
> I found that iflag and xflag can be combined with lflag to reduce the 
> number of redundant local variables, which is a refactoring to improve 
> code readability.Signed-off-by: Wu YiFeng <wuyifeng10@huawei.com>
> 
> Please help me review, thanks.
> 
> ---
>   growfs/xfs_growfs.c | 22 +++++++++++-----------
>   1 file changed, 11 insertions(+), 11 deletions(-)
> 
> diff --git a/growfs/xfs_growfs.c b/growfs/xfs_growfs.c
> index 683961f6..5fb1a9d2 100644
> --- a/growfs/xfs_growfs.c
> +++ b/growfs/xfs_growfs.c
> @@ -8,6 +8,10 @@
>   #include "libfrog/paths.h"
>   #include "libfrog/fsgeom.h"
> 
> +#define LOG_GROW    (1<<0)    /* -l flag: grow log section */
> +#define LOG_EXT2INT    (1<<1)    /* -i flag: convert log from external 
> to internal format */
> +#define LOG_INT2EXT    (1<<2)    /* -x flag: convert log from internal 
> to external format */
> +
>   static void
>   usage(void)
>   {
> @@ -45,7 +49,6 @@ main(int argc, char **argv)
>       long            esize;    /* new rt extent size */
>       int            ffd;    /* mount point file descriptor */
>       struct xfs_fsop_geom    geo;    /* current fs geometry */
> -    int            iflag;    /* -i flag */
>       int            isint;    /* log is currently internal */
>       int            lflag;    /* -l flag */
>       long long        lsize;    /* new log size in fs blocks */
> @@ -55,7 +58,6 @@ main(int argc, char **argv)
>       struct xfs_fsop_geom    ngeo;    /* new fs geometry */
>       int            rflag;    /* -r flag */
>       long long        rsize;    /* new rt size in fs blocks */
> -    int            xflag;    /* -x flag */
>       char            *fname;    /* mount point name */
>       char            *datadev; /* data device name */
>       char            *logdev;  /*  log device name */
> @@ -72,7 +74,7 @@ main(int argc, char **argv)
> 
>       maxpct = esize = 0;
>       dsize = lsize = rsize = 0LL;
> -    aflag = dflag = iflag = lflag = mflag = nflag = rflag = xflag = 0;
> +    aflag = dflag = lflag = mflag = nflag = rflag = 0;
> 
>       while ((c = getopt(argc, argv, "dD:e:ilL:m:np:rR:t:xV")) != EOF) {
>           switch (c) {
> @@ -87,13 +89,13 @@ main(int argc, char **argv)
>               rflag = 1;
>               break;
>           case 'i':
> -            lflag = iflag = 1;
> +            lflag |= LOG_EXT2INT;
>               break;
>           case 'L':
>               lsize = strtoll(optarg, NULL, 10);
>               fallthrough;
>           case 'l':
> -            lflag = 1;
> +            lflag |= LOG_GROW;
>               break;
>           case 'm':
>               mflag = 1;
> @@ -115,7 +117,7 @@ main(int argc, char **argv)
>               mtab_file = optarg;
>               break;
>           case 'x':
> -            lflag = xflag = 1;
> +            lflag |= LOG_INT2EXT;
>               break;
>           case 'V':
>               printf(_("%s version %s\n"), progname, VERSION);
> @@ -124,9 +126,7 @@ main(int argc, char **argv)
>               usage();
>           }
>       }
> -    if (argc - optind != 1)
> -        usage();
> -    if (iflag && xflag)
> +    if (argc - optind != 1 || ((lflag & LOG_EXT2INT) && (lflag & 
> LOG_INT2EXT)))
>           usage();
>       if (dflag + lflag + rflag + mflag == 0)
>           aflag = 1;
> @@ -323,9 +323,9 @@ _("[EXPERIMENTAL] try to shrink unused space %lld, 
> old size is %lld\n"),
> 
>           if (!lsize)
>               lsize = dlsize / (geo.blocksize / BBSIZE);
> -        if (iflag)
> +        if (lflag & LOG_EXT2INT)
>               in.isint = 1;
> -        else if (xflag)
> +        else if (lflag & LOG_INT2EXT)
>               in.isint = 0;
>           else
>               in.isint = xi.logBBsize == 0;

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] xfs_grow: Remove xflag and iflag to reduce redundant temporary variables.
  2023-12-07  8:39 ` [PATCH] xfs_grow: Remove xflag and iflag to reduce redundant temporary variables wuyifeng (C)
  2023-12-08  8:28   ` wuyifeng (C)
@ 2023-12-11 11:10   ` Carlos Maiolino
  1 sibling, 0 replies; 6+ messages in thread
From: Carlos Maiolino @ 2023-12-11 11:10 UTC (permalink / raw)
  To: wuyifeng (C); +Cc: Darrick J. Wong, linux-xfs, louhongxiang

On Thu, Dec 07, 2023 at 04:39:04PM +0800, wuyifeng (C) wrote:
> I found that iflag and xflag can be combined with lflag to reduce the
> number of redundant local variables, which is a refactoring to improve
> code readability.Signed-off-by: Wu YiFeng <wuyifeng10@huawei.com>
> 
> Please help me review, thanks.

Patches should be inlined not submitted as an attachment, please, submit it
correctly, so people will actually look into it.

Same kernel rules apply here:
https://www.kernel.org/doc/html/latest/process/submitting-patches.html#no-mime-no-links-no-compression-no-attachments-just-plain-text

Thanks

Carlos

> 
> ---
>   growfs/xfs_growfs.c | 22 +++++++++++-----------
>   1 file changed, 11 insertions(+), 11 deletions(-)
> 
> diff --git a/growfs/xfs_growfs.c b/growfs/xfs_growfs.c
> index 683961f6..5fb1a9d2 100644
> --- a/growfs/xfs_growfs.c
> +++ b/growfs/xfs_growfs.c
> @@ -8,6 +8,10 @@
>   #include "libfrog/paths.h"
>   #include "libfrog/fsgeom.h"
> 
> +#define LOG_GROW    (1<<0)    /* -l flag: grow log section */
> +#define LOG_EXT2INT    (1<<1)    /* -i flag: convert log from external
> to internal format */
> +#define LOG_INT2EXT    (1<<2)    /* -x flag: convert log from internal
> to external format */
> +
>   static void
>   usage(void)
>   {
> @@ -45,7 +49,6 @@ main(int argc, char **argv)
>       long            esize;    /* new rt extent size */
>       int            ffd;    /* mount point file descriptor */
>       struct xfs_fsop_geom    geo;    /* current fs geometry */
> -    int            iflag;    /* -i flag */
>       int            isint;    /* log is currently internal */
>       int            lflag;    /* -l flag */
>       long long        lsize;    /* new log size in fs blocks */
> @@ -55,7 +58,6 @@ main(int argc, char **argv)
>       struct xfs_fsop_geom    ngeo;    /* new fs geometry */
>       int            rflag;    /* -r flag */
>       long long        rsize;    /* new rt size in fs blocks */
> -    int            xflag;    /* -x flag */
>       char            *fname;    /* mount point name */
>       char            *datadev; /* data device name */
>       char            *logdev;  /*  log device name */
> @@ -72,7 +74,7 @@ main(int argc, char **argv)
> 
>       maxpct = esize = 0;
>       dsize = lsize = rsize = 0LL;
> -    aflag = dflag = iflag = lflag = mflag = nflag = rflag = xflag = 0;
> +    aflag = dflag = lflag = mflag = nflag = rflag = 0;
> 
>       while ((c = getopt(argc, argv, "dD:e:ilL:m:np:rR:t:xV")) != EOF) {
>           switch (c) {
> @@ -87,13 +89,13 @@ main(int argc, char **argv)
>               rflag = 1;
>               break;
>           case 'i':
> -            lflag = iflag = 1;
> +            lflag |= LOG_EXT2INT;
>               break;
>           case 'L':
>               lsize = strtoll(optarg, NULL, 10);
>               fallthrough;
>           case 'l':
> -            lflag = 1;
> +            lflag |= LOG_GROW;
>               break;
>           case 'm':
>               mflag = 1;
> @@ -115,7 +117,7 @@ main(int argc, char **argv)
>               mtab_file = optarg;
>               break;
>           case 'x':
> -            lflag = xflag = 1;
> +            lflag |= LOG_INT2EXT;
>               break;
>           case 'V':
>               printf(_("%s version %s\n"), progname, VERSION);
> @@ -124,9 +126,7 @@ main(int argc, char **argv)
>               usage();
>           }
>       }
> -    if (argc - optind != 1)
> -        usage();
> -    if (iflag && xflag)
> +    if (argc - optind != 1 || ((lflag & LOG_EXT2INT) && (lflag &
> LOG_INT2EXT)))
>           usage();
>       if (dflag + lflag + rflag + mflag == 0)
>           aflag = 1;
> @@ -323,9 +323,9 @@ _("[EXPERIMENTAL] try to shrink unused space %lld,
> old size is %lld\n"),
> 
>           if (!lsize)
>               lsize = dlsize / (geo.blocksize / BBSIZE);
> -        if (iflag)
> +        if (lflag & LOG_EXT2INT)
>               in.isint = 1;
> -        else if (xflag)
> +        else if (lflag & LOG_INT2EXT)
>               in.isint = 0;
>           else
>               in.isint = xi.logBBsize == 0;
> --
> 2.33.0
> 

> From 74c9fe3337a302385999b57ceb819b3439cdbd9c Mon Sep 17 00:00:00 2001
> From: Wu YiFeng <wuyifeng10@huawei.com>
> Date: Thu, 7 Dec 2023 15:47:08 +0800
> Subject: [PATCH] xfs_grow: Remove xflag and iflag to reduce redundant
>  temporary variables.
> 
> Both xflag and iflag are log flags. We can use the bits of lflag to
> indicate all log flags, which is a small code reconstruction.
> 
> Signed-off-by: Wu YiFeng <wuyifeng10@huawei.com>
> ---
>  growfs/xfs_growfs.c | 22 +++++++++++-----------
>  1 file changed, 11 insertions(+), 11 deletions(-)
> 
> diff --git a/growfs/xfs_growfs.c b/growfs/xfs_growfs.c
> index 683961f6..5fb1a9d2 100644
> --- a/growfs/xfs_growfs.c
> +++ b/growfs/xfs_growfs.c
> @@ -8,6 +8,10 @@
>  #include "libfrog/paths.h"
>  #include "libfrog/fsgeom.h"
>  
> +#define LOG_GROW	(1<<0)	/* -l flag: grow log section */
> +#define LOG_EXT2INT	(1<<1)	/* -i flag: convert log from external to internal format */
> +#define LOG_INT2EXT	(1<<2)	/* -x flag: convert log from internal to external format */
> +
>  static void
>  usage(void)
>  {
> @@ -45,7 +49,6 @@ main(int argc, char **argv)
>  	long			esize;	/* new rt extent size */
>  	int			ffd;	/* mount point file descriptor */
>  	struct xfs_fsop_geom	geo;	/* current fs geometry */
> -	int			iflag;	/* -i flag */
>  	int			isint;	/* log is currently internal */
>  	int			lflag;	/* -l flag */
>  	long long		lsize;	/* new log size in fs blocks */
> @@ -55,7 +58,6 @@ main(int argc, char **argv)
>  	struct xfs_fsop_geom	ngeo;	/* new fs geometry */
>  	int			rflag;	/* -r flag */
>  	long long		rsize;	/* new rt size in fs blocks */
> -	int			xflag;	/* -x flag */
>  	char			*fname;	/* mount point name */
>  	char			*datadev; /* data device name */
>  	char			*logdev;  /*  log device name */
> @@ -72,7 +74,7 @@ main(int argc, char **argv)
>  
>  	maxpct = esize = 0;
>  	dsize = lsize = rsize = 0LL;
> -	aflag = dflag = iflag = lflag = mflag = nflag = rflag = xflag = 0;
> +	aflag = dflag = lflag = mflag = nflag = rflag = 0;
>  
>  	while ((c = getopt(argc, argv, "dD:e:ilL:m:np:rR:t:xV")) != EOF) {
>  		switch (c) {
> @@ -87,13 +89,13 @@ main(int argc, char **argv)
>  			rflag = 1;
>  			break;
>  		case 'i':
> -			lflag = iflag = 1;
> +			lflag |= LOG_EXT2INT;
>  			break;
>  		case 'L':
>  			lsize = strtoll(optarg, NULL, 10);
>  			fallthrough;
>  		case 'l':
> -			lflag = 1;
> +			lflag |= LOG_GROW;
>  			break;
>  		case 'm':
>  			mflag = 1;
> @@ -115,7 +117,7 @@ main(int argc, char **argv)
>  			mtab_file = optarg;
>  			break;
>  		case 'x':
> -			lflag = xflag = 1;
> +			lflag |= LOG_INT2EXT;
>  			break;
>  		case 'V':
>  			printf(_("%s version %s\n"), progname, VERSION);
> @@ -124,9 +126,7 @@ main(int argc, char **argv)
>  			usage();
>  		}
>  	}
> -	if (argc - optind != 1)
> -		usage();
> -	if (iflag && xflag)
> +	if (argc - optind != 1 || ((lflag & LOG_EXT2INT) && (lflag & LOG_INT2EXT)))
>  		usage();
>  	if (dflag + lflag + rflag + mflag == 0)
>  		aflag = 1;
> @@ -323,9 +323,9 @@ _("[EXPERIMENTAL] try to shrink unused space %lld, old size is %lld\n"),
>  
>  		if (!lsize)
>  			lsize = dlsize / (geo.blocksize / BBSIZE);
> -		if (iflag)
> +		if (lflag & LOG_EXT2INT)
>  			in.isint = 1;
> -		else if (xflag)
> +		else if (lflag & LOG_INT2EXT)
>  			in.isint = 0;
>  		else
>  			in.isint = xi.logBBsize == 0;
> -- 
> 2.33.0
> 


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] xfs_grow: Remove xflag and iflag to reduce redundant temporary variables.
  2023-12-14  5:00 ` Christoph Hellwig
@ 2023-12-14 17:07   ` Darrick J. Wong
  0 siblings, 0 replies; 6+ messages in thread
From: Darrick J. Wong @ 2023-12-14 17:07 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: wuyifeng (C), Carlos Maiolino, linux-xfs, louhongxiang

On Wed, Dec 13, 2023 at 09:00:39PM -0800, Christoph Hellwig wrote:
> On Thu, Dec 14, 2023 at 10:41:34AM +0800, wuyifeng (C) wrote:
> > Both xflag and iflag are log flags. We can use the bits of lflag to
> > indicate all log flags, which is a small code reconstruction.
> 
> I don't really see much of an upside here.  This now requires me to
> go out of the function to figure out what the flags means, and it adds
> overly long lines making reading the code harder.

Also, lflags is a bitset, so what does (LOG_EXT2INT|LOG_INT2EXT) mean?

--D

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] xfs_grow: Remove xflag and iflag to reduce redundant temporary variables.
  2023-12-14  2:41 wuyifeng (C)
@ 2023-12-14  5:00 ` Christoph Hellwig
  2023-12-14 17:07   ` Darrick J. Wong
  0 siblings, 1 reply; 6+ messages in thread
From: Christoph Hellwig @ 2023-12-14  5:00 UTC (permalink / raw)
  To: wuyifeng (C); +Cc: Carlos Maiolino, Darrick J. Wong, linux-xfs, louhongxiang

On Thu, Dec 14, 2023 at 10:41:34AM +0800, wuyifeng (C) wrote:
> Both xflag and iflag are log flags. We can use the bits of lflag to
> indicate all log flags, which is a small code reconstruction.

I don't really see much of an upside here.  This now requires me to
go out of the function to figure out what the flags means, and it adds
overly long lines making reading the code harder.


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH] xfs_grow: Remove xflag and iflag to reduce redundant temporary variables.
@ 2023-12-14  2:41 wuyifeng (C)
  2023-12-14  5:00 ` Christoph Hellwig
  0 siblings, 1 reply; 6+ messages in thread
From: wuyifeng (C) @ 2023-12-14  2:41 UTC (permalink / raw)
  To: Carlos Maiolino, Darrick J. Wong, linux-xfs; +Cc: louhongxiang

Both xflag and iflag are log flags. We can use the bits of lflag to
indicate all log flags, which is a small code reconstruction.

Signed-off-by: Wu YiFeng <wuyifeng10@huawei.com>
---
 growfs/xfs_growfs.c | 22 +++++++++++-----------
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/growfs/xfs_growfs.c b/growfs/xfs_growfs.c
index 683961f6..5fb1a9d2 100644
--- a/growfs/xfs_growfs.c
+++ b/growfs/xfs_growfs.c
@@ -8,6 +8,10 @@
 #include "libfrog/paths.h"
 #include "libfrog/fsgeom.h"

+#define LOG_GROW	(1<<0)	/* -l flag: grow log section */
+#define LOG_EXT2INT	(1<<1)	/* -i flag: convert log from external to internal format */
+#define LOG_INT2EXT	(1<<2)	/* -x flag: convert log from internal to external format */
+
 static void
 usage(void)
 {
@@ -45,7 +49,6 @@ main(int argc, char **argv)
 	long			esize;	/* new rt extent size */
 	int			ffd;	/* mount point file descriptor */
 	struct xfs_fsop_geom	geo;	/* current fs geometry */
-	int			iflag;	/* -i flag */
 	int			isint;	/* log is currently internal */
 	int			lflag;	/* -l flag */
 	long long		lsize;	/* new log size in fs blocks */
@@ -55,7 +58,6 @@ main(int argc, char **argv)
 	struct xfs_fsop_geom	ngeo;	/* new fs geometry */
 	int			rflag;	/* -r flag */
 	long long		rsize;	/* new rt size in fs blocks */
-	int			xflag;	/* -x flag */
 	char			*fname;	/* mount point name */
 	char			*datadev; /* data device name */
 	char			*logdev;  /*  log device name */
@@ -72,7 +74,7 @@ main(int argc, char **argv)

 	maxpct = esize = 0;
 	dsize = lsize = rsize = 0LL;
-	aflag = dflag = iflag = lflag = mflag = nflag = rflag = xflag = 0;
+	aflag = dflag = lflag = mflag = nflag = rflag = 0;

 	while ((c = getopt(argc, argv, "dD:e:ilL:m:np:rR:t:xV")) != EOF) {
 		switch (c) {
@@ -87,13 +89,13 @@ main(int argc, char **argv)
 			rflag = 1;
 			break;
 		case 'i':
-			lflag = iflag = 1;
+			lflag |= LOG_EXT2INT;
 			break;
 		case 'L':
 			lsize = strtoll(optarg, NULL, 10);
 			fallthrough;
 		case 'l':
-			lflag = 1;
+			lflag |= LOG_GROW;
 			break;
 		case 'm':
 			mflag = 1;
@@ -115,7 +117,7 @@ main(int argc, char **argv)
 			mtab_file = optarg;
 			break;
 		case 'x':
-			lflag = xflag = 1;
+			lflag |= LOG_INT2EXT;
 			break;
 		case 'V':
 			printf(_("%s version %s\n"), progname, VERSION);
@@ -124,9 +126,7 @@ main(int argc, char **argv)
 			usage();
 		}
 	}
-	if (argc - optind != 1)
-		usage();
-	if (iflag && xflag)
+	if (argc - optind != 1 || ((lflag & LOG_EXT2INT) && (lflag & LOG_INT2EXT)))
 		usage();
 	if (dflag + lflag + rflag + mflag == 0)
 		aflag = 1;
@@ -323,9 +323,9 @@ _("[EXPERIMENTAL] try to shrink unused space %lld, old size is %lld\n"),

 		if (!lsize)
 			lsize = dlsize / (geo.blocksize / BBSIZE);
-		if (iflag)
+		if (lflag & LOG_EXT2INT)
 			in.isint = 1;
-		else if (xflag)
+		else if (lflag & LOG_INT2EXT)
 			in.isint = 0;
 		else
 			in.isint = xi.logBBsize == 0;
-- 
2.33.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2023-12-14 17:07 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <0e5P5B3pK_KjP_PgdGLjztYkfjNvEPuXovv9Fz2xm1gNnC0r5NfQf7wOK3OQNZ0GN0yqrL2qgeQpFuZfFv61og==@protonmail.internalid>
2023-12-07  8:39 ` [PATCH] xfs_grow: Remove xflag and iflag to reduce redundant temporary variables wuyifeng (C)
2023-12-08  8:28   ` wuyifeng (C)
2023-12-11 11:10   ` Carlos Maiolino
2023-12-14  2:41 wuyifeng (C)
2023-12-14  5:00 ` Christoph Hellwig
2023-12-14 17:07   ` Darrick J. Wong

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.