Re: [Qemu-devel] [PATCH v2 2/2] configure: disallow spaces and colons in source path and build path

2019-05-09 Thread Peter Maydell
On Mon, 6 May 2019 at 18:27, Eric Blake  wrote:
>
> On 5/3/19 3:27 AM, Antonio Ospite wrote:
> > From: Antonio Ospite 
> >
> > The configure script breaks when the qemu source directory is in a path
> > containing white spaces, in particular the list of targets is not
> > correctly generated when calling "./configure --help" because of how the
> > default_target_list variable is built.
> >
> > In addition to that, *building* qemu from a directory with spaces breaks
> > some assumptions in the Makefiles, even if the original source path does
> > not contain spaces like in the case of an out-of-tree build, or when
> > symlinks are involved.
> >
> > To avoid these issues, refuse to run the configure script and the
> > Makefile if there are spaces or colons in the source path or the build
> > path, taking as inspiration what the kbuild system in linux does.
> >
> > Buglink: https://bugs.launchpad.net/qemu/+bug/1817345
> >
> > Signed-off-by: Antonio Ospite 
> > ---
> >  Makefile  | 4 
> >  configure | 6 ++
> >  2 files changed, 10 insertions(+)
> >
>
> > +++ b/Makefile
> > @@ -1,5 +1,9 @@
> >  # Makefile for QEMU.
> >
> > +ifneq ($(words $(subst :, ,$(CURDIR))), 1)
> > +  $(error main directory cannot contain spaces nor colons)
> > +endif
> > +
> >  # Always point to the root of the build tree (needs GNU make).
> >  BUILD_DIR=$(CURDIR)
> >
> > diff --git a/configure b/configure
> > index 9832cbca5c..f7ad4381bd 100755
> > --- a/configure
> > +++ b/configure
> > @@ -279,6 +279,12 @@ ld_has() {
> >  # make source path absolute
> >  source_path=$(cd "$(dirname -- "$0")"; pwd)
> >
> > +if printf "%s\n" "$source_path" | grep -q "[[:space:]:]" ||
> > +  printf "%s\n" "$PWD" | grep -q "[[:space:]:]";
>
> For less typing and fewer processes, you could shorten this to:
>
> if printf %s\\n "$source_path" "$PWD" | grep -q "[[:space:]:]";
>
> but that's trivial enough for a maintainer to fold in if desired.
>
> Reviewed-by: Eric Blake 

What tree is this going to go in via? I suggest the
-trivial tree.

thanks
-- PMM



Re: [Qemu-devel] [PATCH v2 2/2] configure: disallow spaces and colons in source path and build path

2019-05-06 Thread Antonio Ospite
On Mon, 6 May 2019 12:27:46 -0500
Eric Blake  wrote:

> On 5/3/19 3:27 AM, Antonio Ospite wrote:
> > From: Antonio Ospite 
> > 
> > The configure script breaks when the qemu source directory is in a path
> > containing white spaces, in particular the list of targets is not
> > correctly generated when calling "./configure --help" because of how the
> > default_target_list variable is built.
> > 
> > In addition to that, *building* qemu from a directory with spaces breaks
> > some assumptions in the Makefiles, even if the original source path does
> > not contain spaces like in the case of an out-of-tree build, or when
> > symlinks are involved.
> > 
> > To avoid these issues, refuse to run the configure script and the
> > Makefile if there are spaces or colons in the source path or the build
> > path, taking as inspiration what the kbuild system in linux does.
> > 
> > Buglink: https://bugs.launchpad.net/qemu/+bug/1817345
> > 
> > Signed-off-by: Antonio Ospite 
> > ---
> >  Makefile  | 4 
> >  configure | 6 ++
> >  2 files changed, 10 insertions(+)
> > 
> 
> > +++ b/Makefile
> > @@ -1,5 +1,9 @@
> >  # Makefile for QEMU.
> >  
> > +ifneq ($(words $(subst :, ,$(CURDIR))), 1)
> > +  $(error main directory cannot contain spaces nor colons)
> > +endif
> > +
> >  # Always point to the root of the build tree (needs GNU make).
> >  BUILD_DIR=$(CURDIR)
> >  
> > diff --git a/configure b/configure
> > index 9832cbca5c..f7ad4381bd 100755
> > --- a/configure
> > +++ b/configure
> > @@ -279,6 +279,12 @@ ld_has() {
> >  # make source path absolute
> >  source_path=$(cd "$(dirname -- "$0")"; pwd)
> >  
> > +if printf "%s\n" "$source_path" | grep -q "[[:space:]:]" ||
> > +  printf "%s\n" "$PWD" | grep -q "[[:space:]:]";
> 
> For less typing and fewer processes, you could shorten this to:
> 
> if printf %s\\n "$source_path" "$PWD" | grep -q "[[:space:]:]";
>

I always forget about this printf feature :)

> but that's trivial enough for a maintainer to fold in if desired.
> 
> Reviewed-by: Eric Blake 

Thanks,
   Antonio

-- 
Antonio Ospite
https://ao2.it
https://twitter.com/ao2it

A: Because it messes up the order in which people normally read text.
   See http://en.wikipedia.org/wiki/Posting_style
Q: Why is top-posting such a bad thing?



Re: [Qemu-devel] [PATCH v2 2/2] configure: disallow spaces and colons in source path and build path

2019-05-06 Thread Eric Blake
On 5/3/19 3:27 AM, Antonio Ospite wrote:
> From: Antonio Ospite 
> 
> The configure script breaks when the qemu source directory is in a path
> containing white spaces, in particular the list of targets is not
> correctly generated when calling "./configure --help" because of how the
> default_target_list variable is built.
> 
> In addition to that, *building* qemu from a directory with spaces breaks
> some assumptions in the Makefiles, even if the original source path does
> not contain spaces like in the case of an out-of-tree build, or when
> symlinks are involved.
> 
> To avoid these issues, refuse to run the configure script and the
> Makefile if there are spaces or colons in the source path or the build
> path, taking as inspiration what the kbuild system in linux does.
> 
> Buglink: https://bugs.launchpad.net/qemu/+bug/1817345
> 
> Signed-off-by: Antonio Ospite 
> ---
>  Makefile  | 4 
>  configure | 6 ++
>  2 files changed, 10 insertions(+)
> 

> +++ b/Makefile
> @@ -1,5 +1,9 @@
>  # Makefile for QEMU.
>  
> +ifneq ($(words $(subst :, ,$(CURDIR))), 1)
> +  $(error main directory cannot contain spaces nor colons)
> +endif
> +
>  # Always point to the root of the build tree (needs GNU make).
>  BUILD_DIR=$(CURDIR)
>  
> diff --git a/configure b/configure
> index 9832cbca5c..f7ad4381bd 100755
> --- a/configure
> +++ b/configure
> @@ -279,6 +279,12 @@ ld_has() {
>  # make source path absolute
>  source_path=$(cd "$(dirname -- "$0")"; pwd)
>  
> +if printf "%s\n" "$source_path" | grep -q "[[:space:]:]" ||
> +  printf "%s\n" "$PWD" | grep -q "[[:space:]:]";

For less typing and fewer processes, you could shorten this to:

if printf %s\\n "$source_path" "$PWD" | grep -q "[[:space:]:]";

but that's trivial enough for a maintainer to fold in if desired.

Reviewed-by: Eric Blake 

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3226
Virtualization:  qemu.org | libvirt.org



signature.asc
Description: OpenPGP digital signature