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


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

2019-05-03 Thread Antonio Ospite
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(+)

diff --git a/Makefile b/Makefile
index 1211e78c91..7f3f7a7fef 100644
--- a/Makefile
+++ 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:]:]";
+then
+  error_exit "main directory cannot contain spaces nor colons"
+fi
+
 # default parameters
 cpu=""
 iasl="iasl"
-- 
2.20.1