* Re: [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
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 ` 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