Re: [Qemu-devel] [PATCH v2] win32: Fix CRLF problem in make_device_config.sh

[Blue Swirl]

On Sun, Dec 19, 2010 at 3:42 PM, Stefan Weil <address@hidden> wrote:
> Am 18.12.2010 19:59, schrieb Blue Swirl:
>>
>> Thanks, applied.
>>
>> On Sat, Dec 18, 2010 at 5:09 PM, Andreas Färber <address@hidden>
>> wrote:
>>>
>>> Am 18.12.2010 um 17:34 schrieb Stefan Weil:
>>>
>>>> QEMU source code with CRLF line endings
>>>> which is quite common on windows hosts
>>>> fails with current make_device_config.sh.
>>>>
>>>> The awk script gets the name of the included
>>>> file with \r, so instead of pci.mak it will
>>>> search for pci.mak\r which of course does
>>>> not work.
>>>>
>>>> Fix this by removing any \r.
>>>>
>>>> v2:
>>>>  Avoid using sub() and \r with awk because they are unsupported
>>>>  on some platforms. Use tr to remove \r. This new solution
>>>>  improves portability and was suggested by Paolo Bonzini.
>>>>
>>>> Signed-off-by: Stefan Weil <address@hidden>
>>>
>>> Acked-by: Andreas Färber <address@hidden>
>>>>
>>>> ---
>>>> make_device_config.sh |    2 +-
>>>> 1 files changed, 1 insertions(+), 1 deletions(-)
>>>>
>>>> diff --git a/make_device_config.sh b/make_device_config.sh
>>>> index 8abadfe..596fc5b 100644
>>>> --- a/make_device_config.sh
>>>> +++ b/make_device_config.sh
>>>> @@ -18,7 +18,7 @@ process_includes () {
>>>>
>>>> f=$src
>>>> while [ -n "$f" ] ; do
>>>> -  f=`awk '/^include / {ORS=" " ; print "'$src_dir'/" $2}' $f`
>>>> +  f=`tr -d '\r' < $f | awk '/^include / {ORS=" "; print "'$src_dir'/"
>>>> $2}'`
>>>>  [ $? = 0 ] || exit 1
>>>>  all_includes="$all_includes $f"
>>>> done
>>>> --
>>>> 1.7.2.3
>
> The new code raises a new problem (sorry that I did not detect it earlier):
>
> On hosts with /bin/sh != bash, make displays an error from
> make_device_config.sh:
>
> $ touch default-configs/i386-softmmu.mak
> $ make
>  GEN   i386-softmmu/config-devices.mak
> /qemu/make_device_config.sh: 24: cannot open /qemu/default-configs/pci.mak :
> No such file
>
> make continues, so the error message is not fatal but a cosmetical issue.
> It took me some time to find the reason for this error message although
> it is quite simple:
>
> The filename f which is calculated using awk ends with a blank character
> caused by ORS=" ".
>
> Obviously this blank does not matter for bash and other shells when
> $f is used as a parameter. I/O redirection with bash works, too.
> But dash (and perhaps other simple shells) work different. For dash,
> < $f works like < "$f", so the blank is part of the filename,
> and "pci.mak " of course does not exist. Is this a dash bug or a feature?
>
> Using ORS="" solves the problem for me, but might raise new compatibility
> problems (is an empty records separator always supported?).
>
> Andreas, may I ask you for one more test?
>
> $ echo -e 'include xy\r' | tr -d '\r' | awk '/^include / {ORS=""; print $2}'
> | od -c
>
> It should return 0000002, not 0000003 like the previous test:
>
> 0000000   x   y
> 0000002

How about something like this instead:
(SRC_DIR=/src; echo -e 'include xy' | sed -n 's|^include
*\(.*\)|'$SRC_DIR/'\1|p')
/src/xy