[PATCH] scripts/config: properly report and set string options

[PATCH] scripts/config: properly report and set string options

[Yann E. MORIN]

Currently, scripts/config removes the leading double-quote from
string options, but leaves the trailing double-quote.

Also, double-quotes in a string are escaped, but scripts/config
does not unescape those when printing

Finally, scripts/config does not escape double-quotes when setting
string options.

Eg. the current behavior:
    $ grep -E '^CONFIG_FOO=' .config
    CONFIG_FOO="Bar \"Buz\" Meh"
    $ ./scripts/config -s FOO
    Bar \"Buz\" Meh"
    $ ./scripts/config --set-str FOO 'Alpha "Bravo" Charlie'
    $ grep -E '^CONFIG_FOO=' .config
    CONFIG_FOO="Alpha "Bravo" Charlie"

Fix those three, giving this new behavior:
    $ grep -E '^CONFIG_FOO=' .config
    CONFIG_FOO="Bar \"Buz\" Meh"
    $ ./scripts/config -s FOO
    Bar "Buz" Meh
    $ ./scripts/config --set-str FOO 'Alpha "Bravo" Charlie'
    $ grep -E '^CONFIG_FOO=' .config
    CONFIG_FOO="Alpha \"Bravo\" Charlie"

Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
Cc: stable@vger.kernel.org
Cc: Andi Kleen <andi@firstfloor.org>
Cc: Michal Marek <mmarek@suse.cz>
---
 scripts/config |   11 +++++++----
 1 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/scripts/config b/scripts/config
index a7c7c4b..ed6653e 100755
--- a/scripts/config
+++ b/scripts/config
@@ -107,7 +107,8 @@ while [ "$1" != "" ] ; do
 		;;
 
 	--set-str)
-		set_var "CONFIG_$ARG" "CONFIG_$ARG=\"$1\""
+		# sed swallows one level of escaping, so we need double-escaping
+		set_var "CONFIG_$ARG" "CONFIG_$ARG=\"${1//\"/\\\\\"}\""
 		shift
 		;;
 
@@ -124,9 +125,11 @@ while [ "$1" != "" ] ; do
 			if [ $? != 0 ] ; then
 				echo undef
 			else
-				V="${V/CONFIG_$ARG=/}"
-				V="${V/\"/}"
-				echo "$V"
+				V="${V/#CONFIG_$ARG=/}"
+				V="${V/#\"/}"
+				V="${V/%\"/}"
+				V="${V/\\\"/\"}"
+				echo "${V}"
 			fi
 		fi
 		;;
-- 
1.7.2.5

Re: [PATCH] scripts/config: properly report and set string options

[Yann E. MORIN]

Michal, All,

On Monday 09 April 2012 14:49:10 Yann E. MORIN wrote:
> Currently, scripts/config removes the leading double-quote from
> string options, but leaves the trailing double-quote.
> 
> Also, double-quotes in a string are escaped, but scripts/config
> does not unescape those when printing
> 
> Finally, scripts/config does not escape double-quotes when setting
> string options.

I believe this should also go in for 3.4.

Regards,
Yann E. MORIN.

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'

Re: [PATCH] scripts/config: properly report and set string options

[Andi Kleen]

On Mon, Apr 09, 2012 at 02:49:10PM +0200, Yann E. MORIN wrote:
> Currently, scripts/config removes the leading double-quote from
> string options, but leaves the trailing double-quote.
> 
> Also, double-quotes in a string are escaped, but scripts/config
> does not unescape those when printing
> 
> Finally, scripts/config does not escape double-quotes when setting
> string options.
> Cc: stable@vger.kernel.org

Looks good. 

Acked-by: Andi Kleen <andi@firstfloor.org>

-Andi

Re: [PATCH] scripts/config: properly report and set string options

[Greg KH]

On Mon, Apr 09, 2012 at 02:49:10PM +0200, Yann E. MORIN wrote:
> Currently, scripts/config removes the leading double-quote from
> string options, but leaves the trailing double-quote.
> 
> Also, double-quotes in a string are escaped, but scripts/config
> does not unescape those when printing
> 
> Finally, scripts/config does not escape double-quotes when setting
> string options.
> 
> Eg. the current behavior:
>     $ grep -E '^CONFIG_FOO=' .config
>     CONFIG_FOO="Bar \"Buz\" Meh"
>     $ ./scripts/config -s FOO
>     Bar \"Buz\" Meh"
>     $ ./scripts/config --set-str FOO 'Alpha "Bravo" Charlie'
>     $ grep -E '^CONFIG_FOO=' .config
>     CONFIG_FOO="Alpha "Bravo" Charlie"
> 
> Fix those three, giving this new behavior:
>     $ grep -E '^CONFIG_FOO=' .config
>     CONFIG_FOO="Bar \"Buz\" Meh"
>     $ ./scripts/config -s FOO
>     Bar "Buz" Meh
>     $ ./scripts/config --set-str FOO 'Alpha "Bravo" Charlie'
>     $ grep -E '^CONFIG_FOO=' .config
>     CONFIG_FOO="Alpha \"Bravo\" Charlie"
> 
> Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
> Cc: stable@vger.kernel.org

How does this fit the Documentation/stable_kernel_rules.txt
qualification for a patch to be added to the stable releases?

greg k-h

Re: [PATCH] scripts/config: properly report and set string options

[Yann E. MORIN]

Greg, All,

On Monday 09 April 2012 17:30:06 Greg KH wrote:
> On Mon, Apr 09, 2012 at 02:49:10PM +0200, Yann E. MORIN wrote:
> > Currently, scripts/config removes the leading double-quote from
> > string options, but leaves the trailing double-quote.
> > 
> > Also, double-quotes in a string are escaped, but scripts/config
> > does not unescape those when printing
> > 
> > Finally, scripts/config does not escape double-quotes when setting
> > string options.
[--SNIP--]
> > Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
> > Cc: stable@vger.kernel.org
> 
> How does this fit the Documentation/stable_kernel_rules.txt

Sorry, I was not aware of this list... My bad...
I'll know for next time. Thanks!

> qualification for a patch to be added to the stable releases?

I believe it qualifies because:
  - it is rather trivial
  - it breaks the build for users that tweak the kernel configuration from
    an upper-layer (aka 'integrated') build-system, using this script which
    is bundled with the kernel in the first place.
  - I am a 'real people', and 'it bothers' me ;-)

OTOH, it does not qualify because:
  - there is no Tested-by:
  - I forgot to specify which version it should be applied to
  - obviously, this fix is not yet in Linus' tree

So, please drop from -stable.

If you think that the first item, above, warrants a -stable fix, then I'll
properly re-submit once the patch is in Linus' tree.

Michal, should I re-send an updated patch whithout the CC:stable?

Sorry for the noise.

Regards,
Yann E. MORIN.

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'

Re: [PATCH] scripts/config: properly report and set string options

[Michal Marek]

On 9.4.2012 18:02, Yann E. MORIN wrote:
> Greg, All,
>> How does this fit the Documentation/stable_kernel_rules.txt
> 
> Sorry, I was not aware of this list... My bad...
> I'll know for next time. Thanks!
> 
>> qualification for a patch to be added to the stable releases?
> 
> I believe it qualifies because:
>   - it is rather trivial
>   - it breaks the build for users that tweak the kernel configuration from
>     an upper-layer (aka 'integrated') build-system, using this script which
>     is bundled with the kernel in the first place.
>   - I am a 'real people', and 'it bothers' me ;-)
> 
> OTOH, it does not qualify because:
>   - there is no Tested-by:
>   - I forgot to specify which version it should be applied to
>   - obviously, this fix is not yet in Linus' tree

Of course it's not in Linus' tree. But the point of the Cc: stable line
is that you submit your patch to mainline and don't need to follow up
with an extra submission to stable.


> If you think that the first item, above, warrants a -stable fix, then I'll
> properly re-submit once the patch is in Linus' tree.
> 
> Michal, should I re-send an updated patch whithout the CC:stable?

If Greg does not want this for stable, I will drop the line myself.

Michal

Re: [PATCH] scripts/config: properly report and set string options

[Jonathan Nieder]

Yann E. MORIN wrote:

>   - it breaks the build for users that tweak the kernel configuration from
>     an upper-layer (aka 'integrated') build-system, using this script which
>     is bundled with the kernel in the first place.

The questions I have are, did using special characters like this work
before or is support for values with quotes inside just a new feature?
Or is there some wrapper around the linux makefile that automatically
uses values with quotes on them, making it effectively impossible for
some class of people to run some versions of Linux?

I don't expect this patch to cause problems for people, but it is a
good practice to explain what scenario justifies backporting a patch
to the stable series, if only because it keeps the volume of patches
to search through when there is a new bug down.

Thanks,
Jonathan

Re: [PATCH] scripts/config: properly report and set string options

[Yann E. MORIN]

Jonathan, All,

On Tuesday 10 April 2012 15:57:53 Jonathan Nieder wrote:
> Yann E. MORIN wrote:
> >   - it breaks the build for users that tweak the kernel configuration from
> >     an upper-layer (aka 'integrated') build-system, using this script which
> >     is bundled with the kernel in the first place.
> 
> The questions I have are, did using special characters like this work
> before or is support for values with quotes inside just a new feature?

2.6.29 (first that had this script) allows for escaped double-quotes.
The oldest I tested, 2.6.20, also allowed escaped double-quotes.

> Or is there some wrapper around the linux makefile that automatically
> uses values with quotes on them, making it effectively impossible for
> some class of people to run some versions of Linux?

The scenario involves an upper-layer build system that:
  - is responsible for settings the kernel options based on global
    configuration (eg. the same kernel build for different boards)
    and/or options set by users (eg. I want USB camera support)
  - uses scripts/config to set/unset options rather than implementing
    its own logic
  - in this case the version string can be set to include the user's
    name, which may contain double-quotes (eg. John "Ripper" Smith)

For projects that decided to go with a stable version as the development
base (and won't update for the foreseeable future), then this is a (minor)
inconvenience (although the fix is easily back-ported).

> I don't expect this patch to cause problems for people, but it is a
> good practice to explain what scenario justifies backporting a patch
> to the stable series, if only because it keeps the volume of patches
> to search through when there is a new bug down.

From what I understand from Greg's reply, this patch will be dropped from
stable (and I do understand his position and your concerns).

Regards,
Yann E. MORIN.

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'

Re: [PATCH] scripts/config: properly report and set string options

[Jonathan Nieder]

Yann E. MORIN wrote:

> The scenario involves an upper-layer build system that:
>   - is responsible for settings the kernel options based on global
>     configuration (eg. the same kernel build for different boards)
>     and/or options set by users (eg. I want USB camera support)
>   - uses scripts/config to set/unset options rather than implementing
>     its own logic
>   - in this case the version string can be set to include the user's
>     name, which may contain double-quotes (eg. John "Ripper" Smith)

Thanks for explaining.  That makes sense.

I agree that it doesn't sound like enough of a show-stopper to warrant
inclusion in -stable.  This build system UI bug has existed since
v2.6.31-rc1~335^2~1 (kbuild: add generic --set-str option to
scripts/config, 2009-05-25).

Re: [PATCH] scripts/config: properly report and set string options

[Yann E. MORIN]

Michal, All,

On Monday 09 April 2012 14:49:10 Yann E. MORIN wrote:
> Currently, scripts/config removes the leading double-quote from
> string options, but leaves the trailing double-quote.
> 
> Also, double-quotes in a string are escaped, but scripts/config
> does not unescape those when printing
> 
> Finally, scripts/config does not escape double-quotes when setting
> string options.

Ping? I have not seen this be applied to your tree, neither in for-next,
nor in rc-fixes.

Regards,
Yann E. MORIN.

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'

Re: [PATCH] scripts/config: properly report and set string options

[Yann E. MORIN]

Michal, All,

On Monday 09 April 2012 14:49:10 Yann E. MORIN wrote:
> Currently, scripts/config removes the leading double-quote from
> string options, but leaves the trailing double-quote.
> 
> Also, double-quotes in a string are escaped, but scripts/config
> does not unescape those when printing
> 
> Finally, scripts/config does not escape double-quotes when setting
> string options.

Ping? I have not seen this in your tree yet...

Regards,
Yann E. MORIN.

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'

Re: [PATCH] scripts/config: properly report and set string options

[Michal Marek]

On Thu, Apr 19, 2012 at 11:07:28PM +0200, Yann E. MORIN wrote:
> Michal, All,
> 
> On Monday 09 April 2012 14:49:10 Yann E. MORIN wrote:
> > Currently, scripts/config removes the leading double-quote from
> > string options, but leaves the trailing double-quote.
> > 
> > Also, double-quotes in a string are escaped, but scripts/config
> > does not unescape those when printing
> > 
> > Finally, scripts/config does not escape double-quotes when setting
> > string options.
> 
> Ping? I have not seen this be applied to your tree, neither in for-next,
> nor in rc-fixes.

I applied it to kbuild.git#kconfig now, sorry for the delay.