[PATCH] unifdef: teach it about defined(FOO) syntax

[PATCH] unifdef: teach it about defined(FOO) syntax

[Denys Vlasenko]

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

Hi,

uclibc project patched unifdef.c (which we stole from kernel)
so that it understands defined(FOO) in addition to defined FOO,
and also taught it to understand short-circuited evaluation
of && and ||

The patch is attached. (Sorry, not inline, I fear Google
web mail interface may mangle it).

I ran these commands in unpatched and patched tree:

make ARCH=i386 CROSS_COMPILE=i486-linux-uclibc- defconfig
ln -s asm-x86 include/asm
make ARCH=i386 CROSS_COMPILE=i486-linux-uclibc- headers_install

and then diffed usr/*. The difference clearly shows that
new unifdef works better than old one:


linux-2.6.30.test/usr/include/linux/acct.h:
@@ -59,9 +59,7 @@ struct acct
 	comp_t		ac_majflt;		/* Major Pagefaults */
 	comp_t		ac_swaps;		/* Number of Swaps */
 /* m68k had no padding here. */
-#if !defined(CONFIG_M68K) || !defined(__KERNEL__)
 	__u16		ac_ahz;			/* AHZ */
-#endif
 	__u32		ac_exitcode;		/* Exitcode */
 	char		ac_comm[ACCT_COMM + 1];	/* Command Name */
 	__u8		ac_etime_hi;		/* Elapsed Time MSB */


linux-2.6.30.test/usr/include/linux/soundcard.h:
@@ -1033,7 +1033,6 @@ typedef struct mixer_vol_table {
  */
 #define LOCL_STARTAUDIO		1

-#if !defined(__KERNEL__) || defined(USE_SEQ_MACROS)
 /*
  *	Some convenience macros to simplify programming of the
  *	/dev/sequencer interface
@@ -1275,4 +1274,3 @@ void seqbuf_dump(void);	/* This function
 		(SEQ_DUMPBUF(), write(seqfd, (char*)(patchx), len))

 #endif
-#endif


linux-2.6.30.test/usr/include/linux/videodev.h:
@@ -16,24 +16,6 @@
 #include <linux/ioctl.h>
 #include <linux/videodev2.h>

-#if defined(__MIN_V4L1) && defined (__KERNEL__)
-
-/*
- * Used by those V4L2 core functions that need a minimum V4L1 support,
- * in order to allow V4L1 Compatibilty code compilation.
- */
-
-struct video_mbuf
-{
-	int	size;		/* Total memory to map */
-	int	frames;		/* Frames */
-	int	offsets[VIDEO_MAX_FRAME];
-};
-
-#define VIDIOCGMBUF		_IOR('v',20, struct video_mbuf)		/* Memory map
buffer info */
-
-#else
-#if defined(CONFIG_VIDEO_V4L1_COMPAT) || !defined (__KERNEL__)

 #define VID_TYPE_CAPTURE	1	/* Can capture */
 #define VID_TYPE_TUNER		2	/* Can tune */
@@ -328,8 +310,6 @@ struct video_code
 #define VID_PLAY_RESET			13
 #define VID_PLAY_END_MARK		14

-#endif /* CONFIG_VIDEO_V4L1_COMPAT */
-#endif /* __MIN_V4L1 */

 #endif /* __LINUX_VIDEODEV_H */


+++ linux-2.6.30.test/usr/include/video/edid.h:
@@ -1,13 +1,11 @@
 #ifndef __linux_video_edid_h__
 #define __linux_video_edid_h__

-#if !defined(__KERNEL__) || defined(CONFIG_X86)

 struct edid_info {
 	unsigned char dummy[128];
 };


-#endif

 #endif /* __linux_video_edid_h__ */



Please apply.

Signed-off-by: Denys Vlasenko <vda.linux@googlemail.com>
--
vda

[-- Attachment #2: linux-2.6.30_unifdef.patch --]
[-- Type: text/x-patch, Size: 7047 bytes --]

diff -urpN linux-2.6.30.org/scripts/unifdef.c linux-2.6.30/scripts/unifdef.c
--- linux-2.6.30.org/scripts/unifdef.c	2009-06-10 05:05:27.000000000 +0200
+++ linux-2.6.30/scripts/unifdef.c	2009-06-21 20:05:02.000000000 +0200
@@ -296,6 +296,7 @@ main(int argc, char *argv[])
 		input = stdin;
 	}
 	process();
+	debug("bug at line %d", __LINE__);
 	abort(); /* bug */
 }
 
@@ -438,8 +439,10 @@ done(void)
 static void
 ignoreoff(void)
 {
-	if (depth == 0)
+	if (depth == 0) {
+		debug("bug at line %d", __LINE__);
 		abort(); /* bug */
+	}
 	ignoring[depth] = ignoring[depth-1];
 }
 static void
@@ -471,8 +474,10 @@ nest(void)
 static void
 unnest(void)
 {
-	if (depth == 0)
+	if (depth == 0) {
+		debug("bug at line %d", __LINE__);
 		abort(); /* bug */
+	}
 	depth -= 1;
 }
 static void
@@ -596,8 +601,11 @@ get_line(void)
 				linestate = LS_DIRTY;
 		}
 		/* skipcomment should have changed the state */
-		if (linestate == LS_HASH)
-			abort(); /* bug */
+// Hmm happens sometimes on valid files
+//		if (linestate == LS_HASH) {
+//			debug("bug at line %d", __LINE__);
+//			abort(); /* bug */
+//		}
 	}
 	if (linestate == LS_DIRTY) {
 		while (*cp != '\0')
@@ -649,24 +657,31 @@ static const struct ops {
 	eval_fn *inner;
 	struct op {
 		const char *str;
+		int short_circuit_val;
 		int (*fn)(int, int);
 	} op[5];
 } eval_ops[] = {
-	{ eval_table, { { "||", op_or } } },
-	{ eval_table, { { "&&", op_and } } },
-	{ eval_table, { { "==", op_eq },
-			{ "!=", op_ne } } },
-	{ eval_unary, { { "<=", op_le },
-			{ ">=", op_ge },
-			{ "<", op_lt },
-			{ ">", op_gt } } }
+	{ eval_table, { { "||", 1, op_or } } },
+	{ eval_table, { { "&&", 0, op_and } } },
+	{ eval_table, { { "==", -1, op_eq },
+			{ "!=", -1, op_ne } } },
+	{ eval_unary, { { "<=", -1, op_le },
+			{ ">=", -1, op_ge },
+			{ "<", -1, op_lt },
+			{ ">", -1, op_gt } } }
 };
 
 /*
- * Function for evaluating the innermost parts of expressions,
- * viz. !expr (expr) defined(symbol) symbol number
+ * Function for evaluating the innermost parts of expressions, viz.
+ * "!expr", "(expr)", "defined(symbol)", "defined symbol", "symbol", "number".
  * We reset the keepthis flag when we find a non-constant subexpression.
  */
+// TODO: we use LT_IF both as "I don't know whether it's false or true"
+// (example: "#if defined FOO") and when we see syntax error
+// (example: "#if (1 || 2" - no closing paren!), but this is wrong.
+// Binary && and || need to distinguish these cases in order to handle this:
+// "#if defined KNOWN_UNDEFINED && FOO" - discard
+// "#if defined KNOWN_UNDEFINED && (syntax_error_here" - do not discard!
 static Linetype
 eval_unary(const struct ops *ops, int *valp, const char **cpp)
 {
@@ -678,39 +693,60 @@ eval_unary(const struct ops *ops, int *v
 	if (*cp == '!') {
 		debug("eval%d !", ops - eval_ops);
 		cp++;
-		if (eval_unary(ops, valp, &cp) == LT_IF)
+		if (eval_unary(ops, valp, &cp) == LT_IF) {
+			*cpp = cp;
 			return (LT_IF);
+		}
 		*valp = !*valp;
+
 	} else if (*cp == '(') {
+		Linetype expr_res;
+
 		cp++;
-		debug("eval%d (", ops - eval_ops);
-		if (eval_table(eval_ops, valp, &cp) == LT_IF)
-			return (LT_IF);
+		debug("eval%d (%s", ops - eval_ops, cp);
+		expr_res = eval_table(eval_ops, valp, &cp);
 		cp = skipcomment(cp);
+		*cpp = cp;
 		if (*cp++ != ')')
 			return (LT_IF);
+		*cpp = cp;
+		if (expr_res == LT_IF)
+			return (LT_IF);
+
 	} else if (isdigit((unsigned char)*cp)) {
 		debug("eval%d number", ops - eval_ops);
 		*valp = strtol(cp, &ep, 0);
 		cp = skipsym(cp);
+
 	} else if (strncmp(cp, "defined", 7) == 0 && endsym(cp[7])) {
+		bool parens;
+
 		cp = skipcomment(cp+7);
-		debug("eval%d defined", ops - eval_ops);
-		if (*cp++ != '(')
-			return (LT_IF);
-		cp = skipcomment(cp);
+		debug("eval%d defined '%s'", ops - eval_ops, cp);
+		parens = (*cp == '(');
+		if (parens)
+			cp = skipcomment(cp+1);
 		sym = findsym(cp);
-		if (sym < 0)
-			return (LT_IF);
-		*valp = (value[sym] != NULL);
 		cp = skipsym(cp);
 		cp = skipcomment(cp);
-		if (*cp++ != ')')
+		if (parens) {
+			if (*cp != ')')
+				return (LT_IF);
+			cp = skipcomment(cp+1);
+		}
+		*cpp = cp;
+		if (sym < 0) {
+			debug("sym not found, returning LT_IF");
 			return (LT_IF);
+		}
+		*valp = (value[sym] != NULL);
 		keepthis = false;
+
 	} else if (!endsym(*cp)) {
 		debug("eval%d symbol", ops - eval_ops);
 		sym = findsym(cp);
+		cp = skipsym(cp);
+		*cpp = cp;
 		if (sym < 0)
 			return (LT_IF);
 		if (value[sym] == NULL)
@@ -720,8 +756,8 @@ eval_unary(const struct ops *ops, int *v
 			if (*ep != '\0' || ep == value[sym])
 				return (LT_IF);
 		}
-		cp = skipsym(cp);
 		keepthis = false;
+
 	} else {
 		debug("eval%d bad expr", ops - eval_ops);
 		return (LT_IF);
@@ -738,15 +774,18 @@ eval_unary(const struct ops *ops, int *v
 static Linetype
 eval_table(const struct ops *ops, int *valp, const char **cpp)
 {
+	Linetype left_side;
 	const struct op *op;
 	const char *cp;
 	int val;
 
-	debug("eval%d", ops - eval_ops);
+	debug("eval%d '%s'", ops - eval_ops, *cpp);
+	left_side = ops->inner(ops+1, valp, cpp);
 	cp = *cpp;
-	if (ops->inner(ops+1, valp, &cp) == LT_IF)
-		return (LT_IF);
+
 	for (;;) {
+		Linetype right_side;
+
 		cp = skipcomment(cp);
 		for (op = ops->op; op->str != NULL; op++)
 			if (strncmp(cp, op->str, strlen(op->str)) == 0)
@@ -754,14 +793,37 @@ eval_table(const struct ops *ops, int *v
 		if (op->str == NULL)
 			break;
 		cp += strlen(op->str);
-		debug("eval%d %s", ops - eval_ops, op->str);
-		if (ops->inner(ops+1, &val, &cp) == LT_IF)
+		debug("eval%d '%s'", ops - eval_ops, op->str);
+		right_side = ops->inner(ops+1, &val, &cp);
+		*cpp = cp;
+
+		/* If short_circuit_val is 0 or 1, we can ignore
+		 * right side if left size is known, and its value
+		 * (i.e., *valp) is 0 or !0, respectively */
+		if (left_side != LT_IF && op->short_circuit_val == !!*valp) {
+			debug("op->short_circuit_val:%d *valp:%d cp:'%s'",
+					op->short_circuit_val, *valp, cp);
+			*valp = !!*valp;
+			break;
+		}
+		/* Same for the right side */
+		if (right_side != LT_IF && op->short_circuit_val == !!val) {
+			debug("op->short_circuit_val:%d val:%d cp:'%s'",
+					op->short_circuit_val, val, cp);
+			left_side = right_side;
+			*valp = !!val;
+			break;
+		}
+
+		if (left_side == LT_IF || right_side == LT_IF)
 			return (LT_IF);
 		*valp = op->fn(*valp, val);
+		left_side = right_side;
 	}
 
-	*cpp = cp;
-	debug("eval%d = %d", ops - eval_ops, *valp);
+	debug("eval%d = %d LT_IF:%d", ops - eval_ops, *valp, (left_side == LT_IF));
+	if (left_side == LT_IF)
+		return (LT_IF);
 	return (*valp ? LT_TRUE : LT_FALSE);
 }
 
@@ -779,7 +841,7 @@ ifeval(const char **cpp)
 	debug("eval %s", *cpp);
 	keepthis = killconsts ? false : true;
 	ret = eval_table(eval_ops, &val, cpp);
-	debug("eval = %d", val);
+	debug("val:%d ret:%d keepthis:%d", val, ret, keepthis);
 	return (keepthis ? LT_IF : ret);
 }
 
@@ -886,6 +948,7 @@ skipcomment(const char *cp)
 				incomment = C_COMMENT;
 			continue;
 		default:
+			debug("bug at line %d", __LINE__);
 			abort(); /* bug */
 		}
 	return (cp);

Re: [PATCH] unifdef: teach it about defined(FOO) syntax

[Andrew Morton]

On Sun, 21 Jun 2009 20:21:32 +0200
Denys Vlasenko <vda.linux@googlemail.com> wrote:

> Hi,
> 
> uclibc project patched unifdef.c (which we stole from kernel)
> so that it understands defined(FOO) in addition to defined FOO,
> and also taught it to understand short-circuited evaluation
> of && and ||
> 
> The patch is attached. (Sorry, not inline, I fear Google
> web mail interface may mangle it).
> 
> I ran these commands in unpatched and patched tree:
> 
> make ARCH=i386 CROSS_COMPILE=i486-linux-uclibc- defconfig
> ln -s asm-x86 include/asm
> make ARCH=i386 CROSS_COMPILE=i486-linux-uclibc- headers_install
> 
> and then diffed usr/*. The difference clearly shows that
> new unifdef works better than old one:
> 
> 
> linux-2.6.30.test/usr/include/linux/acct.h:
> @@ -59,9 +59,7 @@ struct acct
>  	comp_t		ac_majflt;		/* Major Pagefaults */
>  	comp_t		ac_swaps;		/* Number of Swaps */
>  /* m68k had no padding here. */
> -#if !defined(CONFIG_M68K) || !defined(__KERNEL__)
>  	__u16		ac_ahz;			/* AHZ */
> -#endif
>  	__u32		ac_exitcode;		/* Exitcode */
>  	char		ac_comm[ACCT_COMM + 1];	/* Command Name */
>  	__u8		ac_etime_hi;		/* Elapsed Time MSB */
> 
> 
> linux-2.6.30.test/usr/include/linux/soundcard.h:
> @@ -1033,7 +1033,6 @@ typedef struct mixer_vol_table {
>   */
>  #define LOCL_STARTAUDIO		1
> 
> -#if !defined(__KERNEL__) || defined(USE_SEQ_MACROS)
>  /*
>   *	Some convenience macros to simplify programming of the
>   *	/dev/sequencer interface
> @@ -1275,4 +1274,3 @@ void seqbuf_dump(void);	/* This function
>  		(SEQ_DUMPBUF(), write(seqfd, (char*)(patchx), len))
> 
>  #endif
> -#endif
> 
> ...
>

eek.  Please avoid quoting anything which looks like a patch in the
changelog.  Because when some poor schmuck comes along and tries to
apply it, patch(1) goes sniffing around in the changelog, thinks it
sees a patch and makes a huge mess of everything.


> Please apply.

Unfortunately unifdef.c got changed after 2.6.30 and this patch throws
more rejects than I am comfortable about fixing.

Re: [PATCH] unifdef: teach it about defined(FOO) syntax

[Sam Ravnborg]

On Fri, Jun 26, 2009 at 03:04:53PM -0700, Andrew Morton wrote:
> On Sun, 21 Jun 2009 20:21:32 +0200
> Denys Vlasenko <vda.linux@googlemail.com> wrote:
> 
> > Hi,
> > 
> > uclibc project patched unifdef.c (which we stole from kernel)
> > so that it understands defined(FOO) in addition to defined FOO,
> > and also taught it to understand short-circuited evaluation
> > of && and ||
> > 
> > The patch is attached. (Sorry, not inline, I fear Google
> > web mail interface may mangle it).
> > 
> > I ran these commands in unpatched and patched tree:
> > 
> > make ARCH=i386 CROSS_COMPILE=i486-linux-uclibc- defconfig
> > ln -s asm-x86 include/asm
> > make ARCH=i386 CROSS_COMPILE=i486-linux-uclibc- headers_install
> > 
> > and then diffed usr/*. The difference clearly shows that
> > new unifdef works better than old one:
> > 
> > 
> > linux-2.6.30.test/usr/include/linux/acct.h:
> > @@ -59,9 +59,7 @@ struct acct
> >  	comp_t		ac_majflt;		/* Major Pagefaults */
> >  	comp_t		ac_swaps;		/* Number of Swaps */
> >  /* m68k had no padding here. */
> > -#if !defined(CONFIG_M68K) || !defined(__KERNEL__)
> >  	__u16		ac_ahz;			/* AHZ */
> > -#endif
> >  	__u32		ac_exitcode;		/* Exitcode */
> >  	char		ac_comm[ACCT_COMM + 1];	/* Command Name */
> >  	__u8		ac_etime_hi;		/* Elapsed Time MSB */
> > 
> > 
> > linux-2.6.30.test/usr/include/linux/soundcard.h:
> > @@ -1033,7 +1033,6 @@ typedef struct mixer_vol_table {
> >   */
> >  #define LOCL_STARTAUDIO		1
> > 
> > -#if !defined(__KERNEL__) || defined(USE_SEQ_MACROS)
> >  /*
> >   *	Some convenience macros to simplify programming of the
> >   *	/dev/sequencer interface
> > @@ -1275,4 +1274,3 @@ void seqbuf_dump(void);	/* This function
> >  		(SEQ_DUMPBUF(), write(seqfd, (char*)(patchx), len))
> > 
> >  #endif
> > -#endif
> > 
> > ...
> >
> 
> eek.  Please avoid quoting anything which looks like a patch in the
> changelog.  Because when some poor schmuck comes along and tries to
> apply it, patch(1) goes sniffing around in the changelog, thinks it
> sees a patch and makes a huge mess of everything.
> 
> 
> > Please apply.
> 
> Unfortunately unifdef.c got changed after 2.6.30 and this patch throws
> more rejects than I am comfortable about fixing.

Russell King actually implemented the same functionality (more or less).
Denys - could you try if the one in the kernel is better/worse
then the one you have here.

The implementation Russell did looks much simpler than this
patch and so far it has only been a win.

	Sam

Re: [PATCH] unifdef: teach it about defined(FOO) syntax

[Denys Vlasenko]

On Saturday 27 June 2009 00:12, Sam Ravnborg wrote:
> > Unfortunately unifdef.c got changed after 2.6.30 and this patch throws
> > more rejects than I am comfortable about fixing.
> 
> Russell King actually implemented the same functionality (more or less).
> Denys - could you try if the one in the kernel is better/worse
> then the one you have here.
> 
> The implementation Russell did looks much simpler than this
> patch and so far it has only been a win.

I checked and linux-2.6.31-rc2 indeed has it fixed.
--
vda