Re: [hackers] [st][PATCH] arg.h: optimize & style

From: Hiltjo Posthuma <hiltjo_AT_codemadness.org>
Date: Sun, 4 Jul 2021 11:55:53 +0200

On Sun, Jul 04, 2021 at 12:30:27PM +0500, Nikita Zlobin wrote:
> - improved code readability
> - cleaned redundant variables and checks
> - isolated reusable part for (E)ARGF
> ---
> arg.h | 65 +++++++++++++++++++++++++++++------------------------------
> 1 file changed, 32 insertions(+), 33 deletions(-)
>
> diff --git a/arg.h b/arg.h
> index a22e019..ee0bbc6 100644
> --- a/arg.h
> +++ b/arg.h
> _AT_@ -9,42 +9,41 @@
> extern char *argv0;
>
> /* use main(int argc, char *argv[]) */
> -#define ARGBEGIN for (argv0 = *argv, argv++, argc--;\
> - argv[0] && argv[0][0] == '-'\
> - && argv[0][1];\
> - argc--, argv++) {\
> - char argc_;\
> - char **argv_;\
> - int brk_;\
> - if (argv[0][1] == '-' && argv[0][2] == '\0') {\
> - argv++;\
> - argc--;\
> - break;\
> - }\
> - int i_;\
> - for (i_ = 1, brk_ = 0, argv_ = argv;\
> - argv[0][i_] && !brk_;\
> - i_++) {\
> - if (argv_ != argv)\
> - break;\
> - argc_ = argv[0][i_];\
> - switch (argc_)
> -
> -#define ARGEND }\
> +#define ARGBEGIN \
> + for (argv0 = *argv, argv++, argc--; \
> + argv[0] && argv[0][0] == '-' && argv[0][1]; \
> + argc--, argv++ ) \
> + { \
> + char arg_; \
> + int brk_; \
> + if (argv[0][1] == '-' && argv[0][2] == '\0') \
> + { \
> + argv++; \
> + argc--; \
> + break; \
> + } \
> + for (brk_ = 0, argv[0]++; \
> + !brk_ && (arg_ = argv[0][0]); \
> + argv[0]++ ) \
> + { \
> + switch (arg_)
> +#define ARGEND \
> + } \
> }
>
> -#define ARGC() argc_
> +#define _ARGF(x) \
> + ( (argv[0][1] == '\0' && argv[1] == NULL) \
> + ? (x) \
> + : (brk_ = 1, ( (++argv[0]) [0] != '\0') \
> + ? argv[0] \
> + : ( argc--, (++argv)[0] ) \
> + ) \
> + )
>
> -#define EARGF(x) ((argv[0][i_+1] == '\0' && argv[1] == NULL)?\
> - ((x), abort(), (char *)0) :\
> - (brk_ = 1, (argv[0][i_+1] != '\0')?\
> - (&argv[0][i_+1]) :\
> - (argc--, argv++, argv[0])))
> +#define ARGC() arg_
>
> -#define ARGF() ((argv[0][i_+1] == '\0' && argv[1] == NULL)?\
> - (char *)0 :\
> - (brk_ = 1, (argv[0][i_+1] != '\0')?\
> - (&argv[0][i_+1]) :\
> - (argc--, argv++, argv[0])))
> +#define EARGF(x) (_ARGF( ((x), abort(), NULL) ))
> +
> +#define ARGF() (_ARGF( NULL ))
>
> #endif
> --
> 2.31.1
>
>

Hi,

Thanks, but I prefer the current style one.

I'm not confident this patch doesn't modify any behaviour.
For example I see the `i` variable was removed, but it is actually important to
not modify argv as this causes issues on NetBSD and OpenBSD process listing
(see commit a5a928bfc1dd049780a45e072cb4ee42de7219bf).

This is just one example. Unless it fixes a bug I rather keep the current code.

-- 
Kind regards,
Hiltjo
Received on Sun Jul 04 2021 - 11:55:53 CEST

This archive was generated by hypermail 2.3.0 : Sun Jul 04 2021 - 12:00:33 CEST