[hackers] [PATCH] Update macros in arg.h to not cause hidden side effects to argv or rely on previously defined variables.

From: sebastien peterson boudreau <sebastien.peterson.boudreau_AT_gmail.com>
Date: Wed, 28 Aug 2024 18:44:53 -0300

 I mainly wanted to gauge interest in this change by sending it. I was
 looking at the macros in arg.h and comparing to the POSIX `getopt`, but
 what I disliked is that `ARGBEGIN` mutates `argv` and relies on the
 variable `argv0` being previously defined. Of course, this is also an
 issue with `getopt` using `optarg` IMO.

 This patch changes the macro to define a new variable, `_argv`,
 although the name is _certainly_ subject to change (especially
 considering we already have `argv_`. I just wanted to make the change
 quick and I figured others could bike-shed about the name). This allows
 the original `argv` argument to `main` to go unchanged. The only other
 change this requires is that `usage` take a string argument to print,
 which can be easily passed when called from `main`.

 I also went ahead and formatted the macro to be a bit easier to
 read just to make making the changes easier on myself. It looks like
 the origin author used tabs to vertically align, rather than spaces (as
 specified in the coding style), so the formatting breaks if you have a
 different tabwidth than them. I believe the formatting I used should
 look equally as correct, regardless of tabwidth.

---
 arg.h | 65 +++++++++++++++++++++++++++++------------------------------
 x.c   | 25 +++++++++++------------
 2 files changed, 44 insertions(+), 46 deletions(-)
diff --git a/arg.h b/arg.h
index a22e019..0726d0a 100644
--- a/arg.h
+++ b/arg.h
_AT_@ -6,45 +6,44 @@
 #ifndef ARG_H__
 #define ARG_H__
 
-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 \
+	char **_argv;\
+	argc--;\
+	for (_argv = argv+1;\
+		_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 ARGC()		argc_
 
-#define EARGF(x)	((argv[0][i_+1] == '\0' && argv[1] == NULL)?\
+#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])))
+				(brk_ = 1, (_argv[0][i_+1] != '\0')?\
+					(&_argv[0][i_+1]) :\
+					(argc--, _argv++, _argv[0])))
 
-#define ARGF()		((argv[0][i_+1] == '\0' && argv[1] == NULL)?\
+#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])))
+				(brk_ = 1, (_argv[0][i_+1] != '\0')?\
+					(&_argv[0][i_+1]) :\
+					(argc--, _argv++, _argv[0])))
 
 #endif
diff --git a/x.c b/x.c
index d73152b..05a3767 100644
--- a/x.c
+++ b/x.c
_AT_@ -15,7 +15,6 @@
 #include <X11/Xft/Xft.h>
 #include <X11/XKBlib.h>
 
-char *argv0;
 #include "arg.h"
 #include "st.h"
 #include "win.h"
_AT_@ -187,7 +186,7 @@ static char *kmap(KeySym, uint);
 static int match(uint, uint);
 
 static void run(void);
-static void usage(void);
+static void usage(char *);
 
 static void (*handler[LASTEvent])(XEvent *) = {
 	[KeyPress] = kpress,
_AT_@ -2024,7 +2023,7 @@ run(void)
 }
 
 void
-usage(void)
+usage(char *argv0)
 {
 	die("usage: %s [-aiv] [-c class] [-f font] [-g geometry]"
 	    " [-n name] [-o file]\n"
_AT_@ -2048,43 +2047,43 @@ main(int argc, char *argv[])
 		allowaltscreen = 0;
 		break;
 	case 'c':
-		opt_class = EARGF(usage());
+		opt_class = EARGF(usage(argv[0]));
 		break;
 	case 'e':
 		if (argc > 0)
 			--argc, ++argv;
 		goto run;
 	case 'f':
-		opt_font = EARGF(usage());
+		opt_font = EARGF(usage(argv[0]));
 		break;
 	case 'g':
-		xw.gm = XParseGeometry(EARGF(usage()),
+		xw.gm = XParseGeometry(EARGF(usage(argv[0])),
 				&xw.l, &xw.t, &cols, &rows);
 		break;
 	case 'i':
 		xw.isfixed = 1;
 		break;
 	case 'o':
-		opt_io = EARGF(usage());
+		opt_io = EARGF(usage(argv[0]));
 		break;
 	case 'l':
-		opt_line = EARGF(usage());
+		opt_line = EARGF(usage(argv[0]));
 		break;
 	case 'n':
-		opt_name = EARGF(usage());
+		opt_name = EARGF(usage(argv[0]));
 		break;
 	case 't':
 	case 'T':
-		opt_title = EARGF(usage());
+		opt_title = EARGF(usage(argv[0]));
 		break;
 	case 'w':
-		opt_embed = EARGF(usage());
+		opt_embed = EARGF(usage(argv[0]));
 		break;
 	case 'v':
-		die("%s " VERSION "\n", argv0);
+		die("%s " VERSION "\n", argv[0]);
 		break;
 	default:
-		usage();
+		usage(argv[0]);
 	} ARGEND;
 
 run:
-- 
2.46.0

Received on Wed Aug 28 2024 - 23:44:53 CEST