| Message ID | CALwxeUvRWYPmwSR4tuH9x8QYwR1AA6qnUarMu=wM-NrK+Aq-6w@mail.gmail.com (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Headers |
Return-Path: <dev-bounces@dpdk.org> X-Original-To: patchwork@dpdk.org Delivered-To: patchwork@dpdk.org Received: from [92.243.14.124] (localhost [IPv6:::1]) by dpdk.org (Postfix) with ESMTP id 6C95F58E8; Fri, 28 Nov 2014 15:06:14 +0100 (CET) Received: from mail-oi0-f43.google.com (mail-oi0-f43.google.com [209.85.218.43]) by dpdk.org (Postfix) with ESMTP id 4A7E7231C for <dev@dpdk.org>; Fri, 28 Nov 2014 15:06:12 +0100 (CET) Received: by mail-oi0-f43.google.com with SMTP id a3so4716361oib.2 for <dev@dpdk.org>; Fri, 28 Nov 2014 06:06:11 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:in-reply-to:references:date :message-id:subject:from:to:cc:content-type; bh=WBBJkwBIB3P197jgrEWkPkbEEPwQoGemH5qT0VfmIvA=; b=FTo3ShrqpkgIslIhhxLJgMp/F+IPr1KISG/r1OvSrgpqz6i67g+DHrMZ0u2VqUH8DC OZU8DL3odLFXggrr12X7ybtRs+zNOQN3nGz9hJaYNqRz6MQv9G8cTaFyu1NMHKjOaQIU 9iE+YgzsY9OIdKybIn77Is3o/2nZfWR0cL6L4CsgZ9vYtNi7msl4LSCLE9KiZdPjSDv4 OR2DlieHbiNmTv121uUvrGMTGP9OXhh/Kliqr6k/0gESoWHXfx29NLASFMG1Qwop8v1X 8nePF7l38X/gViXmmd2NfIsAJCFMf5aIk28s0FQ31m1rPm353Li+MlspfIPV9qp2GYam TJGQ== X-Gm-Message-State: ALoCoQnU/U/oZ8axRwCWEemfL3M4xTrgxDxVWtZYTQD0dWB45NzYKOhe/fRogOUI1AHO+qEbivWF MIME-Version: 1.0 X-Received: by 10.60.94.73 with SMTP id da9mr27794225oeb.10.1417183571534; Fri, 28 Nov 2014 06:06:11 -0800 (PST) Received: by 10.76.172.165 with HTTP; Fri, 28 Nov 2014 06:06:11 -0800 (PST) In-Reply-To: <20141128135604.GB5828@bricha3-MOBL3> References: <1417087745-9004-1-git-send-email-david.marchand@6wind.com> <4256507.tIOyuODinC@xps13> <20141128135604.GB5828@bricha3-MOBL3> Date: Fri, 28 Nov 2014 15:06:11 +0100 Message-ID: <CALwxeUvRWYPmwSR4tuH9x8QYwR1AA6qnUarMu=wM-NrK+Aq-6w@mail.gmail.com> From: David Marchand <david.marchand@6wind.com> To: Bruce Richardson <bruce.richardson@intel.com> Content-Type: text/plain; charset=UTF-8 X-Content-Filtered-By: Mailman/MimeDel 2.1.15 Cc: "dev@dpdk.org" <dev@dpdk.org> Subject: Re: [dpdk-dev] [PATCH] scripts: fix symbol overriding in configuration files X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK <dev.dpdk.org> List-Unsubscribe: <http://dpdk.org/ml/options/dev>, <mailto:dev-request@dpdk.org?subject=unsubscribe> List-Archive: <http://dpdk.org/ml/archives/dev/> List-Post: <mailto:dev@dpdk.org> List-Help: <mailto:dev-request@dpdk.org?subject=help> List-Subscribe: <http://dpdk.org/ml/listinfo/dev>, <mailto:dev-request@dpdk.org?subject=subscribe> Errors-To: dev-bounces@dpdk.org Sender: "dev" <dev-bounces@dpdk.org> |
Commit Message
David Marchand
Nov. 28, 2014, 2:06 p.m. UTC
Hello Bruce, On Fri, Nov 28, 2014 at 2:56 PM, Bruce Richardson < bruce.richardson@intel.com> wrote: > On Thu, Nov 27, 2014 at 10:17:22AM -0800, Thomas Monjalon wrote: > > > When redefining the same symbol in configuration (basically after an > inclusion), > > > we need to undefine the previous symbol to avoid "redefined" errors. > > > > > > Signed-off-by: David Marchand <david.marchand@6wind.com> > > > > Acked-by: Thomas Monjalon <thomas.monjalon@6wind.com> > > > > Applied > > > This patch doesn't seem to work on FreeBSD. I'm still investigating how to > fix > it but right now compilation with gcc/clang on FreeBSD is broken due to the > config.h file having lines like the below in it :-( > > /usr/home/bruce/ > dpdk.org/x86_64-native-bsdapp-clang/include/rte_config.h:3:21: fatal > error: extra tokens at end of #undef directive [-Wextra-tokens] > #undef RTE_EXEC_ENVn#define RTE_EXEC_ENV "bsdapp" > This is probably because of the \n in the sed. Can you try something like this (I did not like this version of my patch at first because it is not that readable) ?
Comments
On Fri, Nov 28, 2014 at 03:06:11PM +0100, David Marchand wrote: > Hello Bruce, > > On Fri, Nov 28, 2014 at 2:56 PM, Bruce Richardson < > bruce.richardson@intel.com> wrote: > > > On Thu, Nov 27, 2014 at 10:17:22AM -0800, Thomas Monjalon wrote: > > > > When redefining the same symbol in configuration (basically after an > > inclusion), > > > > we need to undefine the previous symbol to avoid "redefined" errors. > > > > > > > > Signed-off-by: David Marchand <david.marchand@6wind.com> > > > > > > Acked-by: Thomas Monjalon <thomas.monjalon@6wind.com> > > > > > > Applied > > > > > This patch doesn't seem to work on FreeBSD. I'm still investigating how to > > fix > > it but right now compilation with gcc/clang on FreeBSD is broken due to the > > config.h file having lines like the below in it :-( > > > > /usr/home/bruce/ > > dpdk.org/x86_64-native-bsdapp-clang/include/rte_config.h:3:21: fatal > > error: extra tokens at end of #undef directive [-Wextra-tokens] > > #undef RTE_EXEC_ENVn#define RTE_EXEC_ENV "bsdapp" > > > > This is probably because of the \n in the sed. > > Can you try something like this (I did not like this version of my patch at > first because it is not that readable) ? > > > diff --git a/scripts/gen-config-h.sh b/scripts/gen-config-h.sh > index 2fac08c..d36efd6 100755 > --- a/scripts/gen-config-h.sh > +++ b/scripts/gen-config-h.sh > @@ -35,9 +35,11 @@ echo "#ifndef __RTE_CONFIG_H" > echo "#define __RTE_CONFIG_H" > grep CONFIG_ $1 | > grep -v '^[ \t]*#' | > -sed 's,CONFIG_\(.*\)=y.*$,#undef \1\n#define \1 1,' | > +sed 's,CONFIG_\(.*\)=y.*$,#undef \1\ > +#define \1 1,' | > sed 's,CONFIG_\(.*\)=n.*$,#undef \1,' | > -sed 's,CONFIG_\(.*\)=\(.*\)$,#undef \1\n#define \1 \2,' | > +sed 's,CONFIG_\(.*\)=\(.*\)$,#undef \1\ > +#define \1 \2,' | > sed 's,\# CONFIG_\(.*\) is not set$,#undef \1,' > echo "#endif /* __RTE_CONFIG_H */" > I tried a number of different things to get sed to introduce "\n" characters, but I missed trying that one. I've sent on an alternative fix now, which uses tr instead of sed to insert the "\n"'s. It's not a fix I'm terribly happy with, but having seen the above (but not tried it yet), it actually doesn't seem that bad in comparison :-) /Bruce
On Fri, Nov 28, 2014 at 03:06:11PM +0100, David Marchand wrote: > Hello Bruce, > > On Fri, Nov 28, 2014 at 2:56 PM, Bruce Richardson < > bruce.richardson@intel.com> wrote: > > > On Thu, Nov 27, 2014 at 10:17:22AM -0800, Thomas Monjalon wrote: > > > > When redefining the same symbol in configuration (basically after an > > inclusion), > > > > we need to undefine the previous symbol to avoid "redefined" errors. > > > > > > > > Signed-off-by: David Marchand <david.marchand@6wind.com> > > > > > > Acked-by: Thomas Monjalon <thomas.monjalon@6wind.com> > > > > > > Applied > > > > > This patch doesn't seem to work on FreeBSD. I'm still investigating how to > > fix > > it but right now compilation with gcc/clang on FreeBSD is broken due to the > > config.h file having lines like the below in it :-( > > > > /usr/home/bruce/ > > dpdk.org/x86_64-native-bsdapp-clang/include/rte_config.h:3:21: fatal > > error: extra tokens at end of #undef directive [-Wextra-tokens] > > #undef RTE_EXEC_ENVn#define RTE_EXEC_ENV "bsdapp" > > > > This is probably because of the \n in the sed. > > Can you try something like this (I did not like this version of my patch at > first because it is not that readable) ? > > > diff --git a/scripts/gen-config-h.sh b/scripts/gen-config-h.sh > index 2fac08c..d36efd6 100755 > --- a/scripts/gen-config-h.sh > +++ b/scripts/gen-config-h.sh > @@ -35,9 +35,11 @@ echo "#ifndef __RTE_CONFIG_H" > echo "#define __RTE_CONFIG_H" > grep CONFIG_ $1 | > grep -v '^[ \t]*#' | > -sed 's,CONFIG_\(.*\)=y.*$,#undef \1\n#define \1 1,' | > +sed 's,CONFIG_\(.*\)=y.*$,#undef \1\ > +#define \1 1,' | > sed 's,CONFIG_\(.*\)=n.*$,#undef \1,' | > -sed 's,CONFIG_\(.*\)=\(.*\)$,#undef \1\n#define \1 \2,' | > +sed 's,CONFIG_\(.*\)=\(.*\)$,#undef \1\ > +#define \1 \2,' | > sed 's,\# CONFIG_\(.*\) is not set$,#undef \1,' > echo "#endif /* __RTE_CONFIG_H */" > > This proposed fix also works. I don't mind whether my patch proposal or this fix gets applied - gen-config-h.sh is not a commonly modified script anyway. /Bruce
On Fri, Nov 28, 2014 at 3:43 PM, Bruce Richardson < bruce.richardson@intel.com> wrote: > This proposed fix also works. I don't mind whether my patch proposal or > this > fix gets applied - gen-config-h.sh is not a commonly modified script > anyway. > I prefer my ugliness ;-) But Thomas just proposed me something that could work. Trying ...
On Fri, Nov 28, 2014 at 03:49:29PM +0100, David Marchand wrote: > On Fri, Nov 28, 2014 at 3:43 PM, Bruce Richardson < > bruce.richardson@intel.com> wrote: > > > This proposed fix also works. I don't mind whether my patch proposal or > > this > > fix gets applied - gen-config-h.sh is not a commonly modified script > > anyway. > > > > I prefer my ugliness ;-) > But Thomas just proposed me something that could work. > Trying ... > Yes, it's ugly, but it's probably more resilient. I'm looking forward to getting an option C. /Bruce
On Fri, Nov 28, 2014 at 3:59 PM, Bruce Richardson < bruce.richardson@intel.com> wrote: > Yes, it's ugly, but it's probably more resilient. I'm looking forward to > getting > an option C. > Option C and D are ugly as well (using some bashism like $' ' or using an intermediate variable with a newline in it). Our "posix" guy told me that he prefers the initial version of the patch. I will send a patch with this. Thomas ?
diff --git a/scripts/gen-config-h.sh b/scripts/gen-config-h.sh index 2fac08c..d36efd6 100755 --- a/scripts/gen-config-h.sh +++ b/scripts/gen-config-h.sh @@ -35,9 +35,11 @@ echo "#ifndef __RTE_CONFIG_H" echo "#define __RTE_CONFIG_H" grep CONFIG_ $1 | grep -v '^[ \t]*#' | -sed 's,CONFIG_\(.*\)=y.*$,#undef \1\n#define \1 1,' | +sed 's,CONFIG_\(.*\)=y.*$,#undef \1\ +#define \1 1,' | sed 's,CONFIG_\(.*\)=n.*$,#undef \1,' | -sed 's,CONFIG_\(.*\)=\(.*\)$,#undef \1\n#define \1 \2,' | +sed 's,CONFIG_\(.*\)=\(.*\)$,#undef \1\ +#define \1 \2,' | sed 's,\# CONFIG_\(.*\) is not set$,#undef \1,' echo "#endif /* __RTE_CONFIG_H */"