Re: [PATCH] kvm: external module: fix unifdef problem

[Sheng Yang <sheng@linux.intel.com>]

On Saturday 25 October 2008 02:10:54 Hollis Blanchard wrote:
> On Thu, 2008-10-23 at 06:41 +0000, Avi Kivity wrote:
> > From: Sheng Yang <sheng@linux.intel.com>
> >
> > unifdef needs a -U parameter to deal with undefined macros, otherwise
> > it can't deal with #if defined() || defined().
> >
> > Also fix a historic bug on never execute unifdef...
> >
> > Also discard "set -e" before unifdef, because unifdef would return 1 if
> > it have done something to the file.
> >
> > Signed-off-by: Sheng Yang <sheng@linux.intel.com>
> > Signed-off-by: Avi Kivity <avi@redhat.com>
> >
> > diff --git a/kernel/Makefile b/kernel/Makefile
> > index ef18fa6..fed3bd4 100644
> > --- a/kernel/Makefile
> > +++ b/kernel/Makefile
> > @@ -3,6 +3,8 @@ include config.kbuild
> >
> >  ARCH_DIR = $(if $(filter $(ARCH),x86_64 i386),x86,$(ARCH))
> >  ARCH_CONFIG := $(shell echo $(ARCH_DIR) | tr '[:lower:]' '[:upper:]')
> > +# NONARCH_CONFIG used for unifdef, and only cover X86 and IA64 now
> > +NONARCH_CONFIG = $(filter-out $(ARCH_CONFIG),X86 IA64)
> >
> >  KVERREL = $(patsubst /lib/modules/%/build,%,$(KERNELDIR))
> >
> > @@ -24,8 +26,8 @@ _hack = mv $1 $1.orig && \
> >
> >  	    | sed '/\#include/! s/\blapic\b/l_apic/g' > $1 && rm $1.orig
> >
> >  unifdef = mv $1 $1.orig && \
> > -	  unifdef -DCONFIG_$(ARCH_CONFIG) $1.orig > $1; \
> > -          [ $$? -le 1 ] && rm $1.orig
> > +	  unifdef -DCONFIG_$(ARCH_CONFIG) -UCONFIG_$(NONARCH_CONFIG) $1.orig >
> > $1; \ +          [ $$? -le 2 ] && rm $1.orig
> >
> >  hack = $(call _hack,$T/$(strip $1))
>
> Guys, I don't mind if you add new things that aren't enabled for other
> architectures, but please try to be a little more careful about breaking
> us.

Oh, sorry. I've informed the Christian Ehrhardt to checked the issue, but he 
is too busy at the time... I would be more careful next time...

Sorry for the inconvenient...
-- 
regards
Yang, Sheng
>
> This patch results in the following on PowerPC:
>         mv $i $i.orig && unifdef -DCONFIG_POWERPC -UCONFIG_X86 IA64
>         $i.orig > $i; [ $? -le 2 ] && rm $i.orig; done
>         unifdef: can only do one file
>
> Here's my proposed fix:
>
> kvm: external module: Treat NONARCH_CONFIG as a list, not a single item.
>
> Signed-off-by: Hollis Blanchard <hollisb@us.ibm.com>
>
> diff --git a/kernel/Makefile b/kernel/Makefile
> --- a/kernel/Makefile
> +++ b/kernel/Makefile
> @@ -25,8 +25,9 @@ _hack = mv $1 $1.orig && \
>  	gawk -v version=$(version) -f $(ARCH_DIR)/hack-module.awk $1.orig \
>
>  	    | sed '/\#include/! s/\blapic\b/l_apic/g' > $1 && rm $1.orig
>
> +unifdef_uflags = $(foreach arch, $(NONARCH_CONFIG), -UCONFIG_$(arch))
>  unifdef = mv $1 $1.orig && \
> -	  unifdef -DCONFIG_$(ARCH_CONFIG) -UCONFIG_$(NONARCH_CONFIG) $1.orig >
> $1; \ +	  unifdef -DCONFIG_$(ARCH_CONFIG) $(unifdef_uflags) $1.orig > $1; \
> [ $$? -le 2 ] && rm $1.orig
>
>  hack = $(call _hack,$T/$(strip $1))
>
> Comments?