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

[Laurent Vivier <laurent@vivier.eu>]

On 22/05/2019 17:01, Antonio Ospite wrote:
> On 22/05/19 15:57, Laurent Vivier wrote:
>> On 09/05/2019 16:42, Peter Maydell wrote:
>>> On Mon, 6 May 2019 at 18:27, Eric Blake <eblake@redhat.com> wrote:
>>>>
>>>> On 5/3/19 3:27 AM, Antonio Ospite wrote:
>>>>> From: Antonio Ospite <antonio.ospite@collabora.com>
>>>>>
>>>>> 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 <antonio.ospite@collabora.com>
>>>>> ---
>>>>>   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 <eblake@redhat.com>
>>>
>>> What tree is this going to go in via? I suggest the
>>> -trivial tree.
>>
>>
>> Applied (unchanged) to my trivial-patches branch.
>>
> 
> Thank you Laurent.
> 
> I'll think about sending a followup patch with the changes proposed by 
> Eric and I'll CC you if I do.

If you want to send a v3 of this patch to update it, I can wait.

Thanks,
Laurent