[Buildroot] [PATCH] fs/ext2: fix double quoted label

[Buildroot] [PATCH] fs/ext2: fix double quoted label

[Gaël PORTAY]

Hi all,

Since the commit 6dd7bbb59134799ed3d7343f238b3b02592faebf, the label does
not need anymore to be quoted. Even worse it *must* not be simple-quoted,
unless the label will contain the double-quotes from the config variable
BR2_TARGET_ROOTFS_EXT2_LABEL.

The commit mentionned above has replaced echo by printf:
-	echo "$$(ROOTFS_$(2)_CMD)" >> $$(FAKEROOT_SCRIPT)
+	$$(call PRINTF,$$(ROOTFS_$(2)_CMD)) >> $$(FAKEROOT_SCRIPT)

Since this commit the rootfs label contains extra double-quotes.
$ blkid
/dev/mmcblk0: LABEL=""BR 2016.08"" UUID="xxx"
                     ^          ^

With this fix, the extra double-quotes have disappeared:
# blkid 
/dev/mmcblk0: LABEL="BR 2016.11-rc2" UUID="yyy"

Note: I tested the fix under pseudo and fakeroot, even if has nothing
to deal with this those tools.

Regards,
Ga?l PORTAY (1):
  fs/ext2: fix double quoted label

 fs/ext2/ext2.mk | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

-- 
2.10.2

[Buildroot] [PATCH] fs/ext2: fix double quoted label

[Gaël PORTAY]

Since the commit 6dd7bbb59134799ed3d7343f238b3b02592faebf, the label does
not need anymore to be quoted. Even worse it *must* not be simple-quoted,
unless the label will contain the double-quotes from the config variable
BR2_TARGET_ROOTFS_EXT2_LABEL.

Signed-off-by: Ga?l PORTAY <gael.portay@savoirfairelinux.com>
---
 fs/ext2/ext2.mk | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/fs/ext2/ext2.mk b/fs/ext2/ext2.mk
index 7417f81..49b44e4 100644
--- a/fs/ext2/ext2.mk
+++ b/fs/ext2/ext2.mk
@@ -20,12 +20,8 @@ ifneq ($(strip $(BR2_TARGET_ROOTFS_EXT2_RESBLKS)),0)
 EXT2_OPTS += -r $(BR2_TARGET_ROOTFS_EXT2_RESBLKS)
 endif
 
-# Not qstrip-ing the variable, because it may contain spaces, but we must
-# qstrip it when checking. Furthermore, we need to further quote it, so
-# that the quotes do not get eaten by the echo statement when creating the
-# fakeroot script
 ifneq ($(call qstrip,$(BR2_TARGET_ROOTFS_EXT2_LABEL)),)
-EXT2_OPTS += -l '$(BR2_TARGET_ROOTFS_EXT2_LABEL)'
+EXT2_OPTS += -l $(BR2_TARGET_ROOTFS_EXT2_LABEL)
 endif
 
 ROOTFS_EXT2_DEPENDENCIES = host-mke2img
-- 
2.10.2

[Buildroot] [PATCH] fs/ext2: fix double quoted label

[Arnout Vandecappelle]

On 16-11-16 20:55, Ga?l PORTAY wrote:
> Since the commit 6dd7bbb59134799ed3d7343f238b3b02592faebf, the label does
> not need anymore to be quoted. Even worse it *must* not be simple-quoted,
> unless the label will contain the double-quotes from the config variable
> BR2_TARGET_ROOTFS_EXT2_LABEL.
> 
> Signed-off-by: Ga?l PORTAY <gael.portay@savoirfairelinux.com>
> ---
>  fs/ext2/ext2.mk | 6 +-----
>  1 file changed, 1 insertion(+), 5 deletions(-)
> 
> diff --git a/fs/ext2/ext2.mk b/fs/ext2/ext2.mk
> index 7417f81..49b44e4 100644
> --- a/fs/ext2/ext2.mk
> +++ b/fs/ext2/ext2.mk
> @@ -20,12 +20,8 @@ ifneq ($(strip $(BR2_TARGET_ROOTFS_EXT2_RESBLKS)),0)
>  EXT2_OPTS += -r $(BR2_TARGET_ROOTFS_EXT2_RESBLKS)
>  endif
>  
> -# Not qstrip-ing the variable, because it may contain spaces, but we must
> -# qstrip it when checking. Furthermore, we need to further quote it, so
> -# that the quotes do not get eaten by the echo statement when creating the
> -# fakeroot script
>  ifneq ($(call qstrip,$(BR2_TARGET_ROOTFS_EXT2_LABEL)),)
> -EXT2_OPTS += -l '$(BR2_TARGET_ROOTFS_EXT2_LABEL)'
> +EXT2_OPTS += -l $(BR2_TARGET_ROOTFS_EXT2_LABEL)

 We usually don't rely on the quotes added by Kconfig, because it's possible
that the variable is overridden on the command line like
make BR2_TARGET_ROOTFS_EXT2_LABEL="foo bar"

So it should be
EXT2_OPTS += -l '$(call qstrip,$(BR2_TARGET_ROOTFS_EXT2_LABEL))'

 Regards,
 Arnout

>  endif
>  
>  ROOTFS_EXT2_DEPENDENCIES = host-mke2img
> 

-- 
Arnout Vandecappelle                          arnout at mind be
Senior Embedded Software Architect            +32-16-286500
Essensium/Mind                                http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium           BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint:  7493 020B C7E3 8618 8DEC 222C 82EB F404 F9AC 0DDF

[Buildroot] [PATCH] fs/ext2: fix double quoted label

[Gaël PORTAY]

Hi Arnout,

I am sorry, I saw your email shortly afterwards sending the v2...

On Wed, Nov 16, 2016 at 11:58:23PM +0100, Arnout Vandecappelle wrote:
> 
> 
> On 16-11-16 20:55, Ga?l PORTAY wrote:
> > Since the commit 6dd7bbb59134799ed3d7343f238b3b02592faebf, the label does
> > not need anymore to be quoted. Even worse it *must* not be simple-quoted,
> > unless the label will contain the double-quotes from the config variable
> > BR2_TARGET_ROOTFS_EXT2_LABEL.
> > 
> > Signed-off-by: Ga?l PORTAY <gael.portay@savoirfairelinux.com>
> > ---
> >  fs/ext2/ext2.mk | 6 +-----
> >  1 file changed, 1 insertion(+), 5 deletions(-)
> > 
> > diff --git a/fs/ext2/ext2.mk b/fs/ext2/ext2.mk
> > index 7417f81..49b44e4 100644
> > --- a/fs/ext2/ext2.mk
> > +++ b/fs/ext2/ext2.mk
> > @@ -20,12 +20,8 @@ ifneq ($(strip $(BR2_TARGET_ROOTFS_EXT2_RESBLKS)),0)
> >  EXT2_OPTS += -r $(BR2_TARGET_ROOTFS_EXT2_RESBLKS)
> >  endif
> >  
> > -# Not qstrip-ing the variable, because it may contain spaces, but we must
> > -# qstrip it when checking. Furthermore, we need to further quote it, so
> > -# that the quotes do not get eaten by the echo statement when creating the
> > -# fakeroot script
> >  ifneq ($(call qstrip,$(BR2_TARGET_ROOTFS_EXT2_LABEL)),)
> > -EXT2_OPTS += -l '$(BR2_TARGET_ROOTFS_EXT2_LABEL)'
> > +EXT2_OPTS += -l $(BR2_TARGET_ROOTFS_EXT2_LABEL)
> 
>  We usually don't rely on the quotes added by Kconfig, because it's possible
> that the variable is overridden on the command line like
> make BR2_TARGET_ROOTFS_EXT2_LABEL="foo bar"
> 

For sure you are right, it was not not appropriate to use the Kconfig variable.

> So it should be
> EXT2_OPTS += -l '$(call qstrip,$(BR2_TARGET_ROOTFS_EXT2_LABEL))'
> 

As discuss with Thomas and Yann on the channel, we moved to this solution:

EXT2_LABEL := $(subst ",,$(BR2_TARGET_ROOTFS_EXT2_LABEL))
ifneq ($(EXT2_LABEL),)
EXT2_OPTS += -l "$(EXT2_LABEL)"
endif

... to not rely on the quotes of Kconfig (as you said) and to preserved the
spaces from the Kconfig variable ("BR   2016-11-rc2").

>  Regards,
>  Arnout
> 
> >  endif
> >  
> >  ROOTFS_EXT2_DEPENDENCIES = host-mke2img
> > 
> 
> -- 
> Arnout Vandecappelle                          arnout at mind be
> Senior Embedded Software Architect            +32-16-286500
> Essensium/Mind                                http://www.mind.be
> G.Geenslaan 9, 3001 Leuven, Belgium           BE 872 984 063 RPR Leuven
> LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
> GPG fingerprint:  7493 020B C7E3 8618 8DEC 222C 82EB F404 F9AC 0DDF

Regards,
Ga?l

[Buildroot] [PATCH] fs/ext2: fix double quoted label

[Gaël PORTAY]

Hi Thomas,

On Mon, Nov 21, 2016 at 09:15:28PM +0100, Thomas Petazzoni wrote:
> Hello,
> 
> On Mon, 21 Nov 2016 11:15:29 -0500, Ga?l PORTAY wrote:
> > Since the commit 6dd7bbb59134799ed3d7343f238b3b02592faebf, the label does
> > not need anymore to be quoted. Even worse it *must* not be simple-quoted,
> > unless the label will contain the double-quotes from the config variable
> > BR2_TARGET_ROOTFS_EXT2_LABEL.
> > 
> > The commit mentionned above has replaced echo by printf:
> > -	echo "$$(ROOTFS_$(2)_CMD)" >> $$(FAKEROOT_SCRIPT)
> > +	$$(call PRINTF,$$(ROOTFS_$(2)_CMD)) >> $$(FAKEROOT_SCRIPT)
> > 
> > Since this commit the rootfs label contains extra double-quotes.
> > $ blkid
> > /dev/mmcblk0: LABEL=""BR 2016.08"" UUID="xxx"
> >                      ^          ^
> > 
> > With this fix, the extra double-quotes have disappeared:
> > /dev/mmcblk0: LABEL="BR 2016.11-rc2" UUID="yyy"
> > 
> > Signed-off-by: Ga?l PORTAY <gael.portay@savoirfairelinux.com>
> > Acked-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
> > ---
> >  fs/ext2/ext2.mk | 11 +++++------
> >  1 file changed, 5 insertions(+), 6 deletions(-)
> 
> Applied to master, thanks.
> 
> Thomas
> -- 
> Thomas Petazzoni, CTO, Free Electrons
> Embedded Linux, Kernel and Android engineering
> http://free-electrons.com

Thanks

[Buildroot] [PATCH] fs/ext2: fix double quoted label

[Thomas Petazzoni]

Hello,

On Mon, 21 Nov 2016 11:15:29 -0500, Ga?l PORTAY wrote:
> Since the commit 6dd7bbb59134799ed3d7343f238b3b02592faebf, the label does
> not need anymore to be quoted. Even worse it *must* not be simple-quoted,
> unless the label will contain the double-quotes from the config variable
> BR2_TARGET_ROOTFS_EXT2_LABEL.
> 
> The commit mentionned above has replaced echo by printf:
> -	echo "$$(ROOTFS_$(2)_CMD)" >> $$(FAKEROOT_SCRIPT)
> +	$$(call PRINTF,$$(ROOTFS_$(2)_CMD)) >> $$(FAKEROOT_SCRIPT)
> 
> Since this commit the rootfs label contains extra double-quotes.
> $ blkid
> /dev/mmcblk0: LABEL=""BR 2016.08"" UUID="xxx"
>                      ^          ^
> 
> With this fix, the extra double-quotes have disappeared:
> /dev/mmcblk0: LABEL="BR 2016.11-rc2" UUID="yyy"
> 
> Signed-off-by: Ga?l PORTAY <gael.portay@savoirfairelinux.com>
> Acked-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
> ---
>  fs/ext2/ext2.mk | 11 +++++------
>  1 file changed, 5 insertions(+), 6 deletions(-)

Applied to master, thanks.

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[Buildroot] [PATCH] fs/ext2: fix double quoted label

[Gaël PORTAY]

Since the commit 6dd7bbb59134799ed3d7343f238b3b02592faebf, the label does
not need anymore to be quoted. Even worse it *must* not be simple-quoted,
unless the label will contain the double-quotes from the config variable
BR2_TARGET_ROOTFS_EXT2_LABEL.

The commit mentionned above has replaced echo by printf:
-	echo "$$(ROOTFS_$(2)_CMD)" >> $$(FAKEROOT_SCRIPT)
+	$$(call PRINTF,$$(ROOTFS_$(2)_CMD)) >> $$(FAKEROOT_SCRIPT)

Since this commit the rootfs label contains extra double-quotes.
$ blkid
/dev/mmcblk0: LABEL=""BR 2016.08"" UUID="xxx"
                     ^          ^

With this fix, the extra double-quotes have disappeared:
/dev/mmcblk0: LABEL="BR 2016.11-rc2" UUID="yyy"

Signed-off-by: Ga?l PORTAY <gael.portay@savoirfairelinux.com>
Acked-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
---
 fs/ext2/ext2.mk | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/fs/ext2/ext2.mk b/fs/ext2/ext2.mk
index 7417f81..cfb34c4 100644
--- a/fs/ext2/ext2.mk
+++ b/fs/ext2/ext2.mk
@@ -20,12 +20,11 @@ ifneq ($(strip $(BR2_TARGET_ROOTFS_EXT2_RESBLKS)),0)
 EXT2_OPTS += -r $(BR2_TARGET_ROOTFS_EXT2_RESBLKS)
 endif
 
-# Not qstrip-ing the variable, because it may contain spaces, but we must
-# qstrip it when checking. Furthermore, we need to further quote it, so
-# that the quotes do not get eaten by the echo statement when creating the
-# fakeroot script
-ifneq ($(call qstrip,$(BR2_TARGET_ROOTFS_EXT2_LABEL)),)
-EXT2_OPTS += -l '$(BR2_TARGET_ROOTFS_EXT2_LABEL)'
+# qstrip results in stripping consecutive spaces into a single one. So the
+# variable is not qstrip-ed to preserve the integrity of the string value.
+EXT2_LABEL := $(subst ",,$(BR2_TARGET_ROOTFS_EXT2_LABEL))
+ifneq ($(EXT2_LABEL),)
+EXT2_OPTS += -l "$(EXT2_LABEL)"
 endif
 
 ROOTFS_EXT2_DEPENDENCIES = host-mke2img
-- 
2.10.2