| Message ID | AANLkTimyhrj9NG0vPTJv9oJyUN+UBdyos6wBDR9Mcss_@mail.gmail.com |
|---|---|
| State | New |
| Headers | show |
Am 12.09.2010 um 19:47 schrieb Blue Swirl: > On Sun, Sep 12, 2010 at 5:35 PM, Andreas Färber <andreas.faerber@web.de > > wrote: >> Am 12.09.2010 um 19:22 schrieb Blue Swirl: >> >>> What is the output of "sh ./tracetool --nop --check-backend"? >> >> ./tracetool: syntax error at line 51: `$' unexpected > > Does this patch fix the problem? > > diff --git a/tracetool b/tracetool > index 534cc70..c7582bf 100755 > --- a/tracetool > +++ b/tracetool > @@ -48,7 +48,8 @@ get_argnames() > { > local nfields field name > nfields=0 > - for field in $(get_args "$1"); do > + args=get_args "$1" > + for field in "$args"; do This part yes. (I took the liberty of adding args to the local line above) > nfields=$((nfields + 1)) Next error is here: ./tracetool: syntax error at line 53: `nfields=$' unexpected
On Sun, Sep 12, 2010 at 5:58 PM, Andreas Färber <andreas.faerber@web.de> wrote: > Am 12.09.2010 um 19:47 schrieb Blue Swirl: > >> On Sun, Sep 12, 2010 at 5:35 PM, Andreas Färber <andreas.faerber@web.de> >> wrote: >>> >>> Am 12.09.2010 um 19:22 schrieb Blue Swirl: >>> >>>> What is the output of "sh ./tracetool --nop --check-backend"? >>> >>> ./tracetool: syntax error at line 51: `$' unexpected >> >> Does this patch fix the problem? >> >> diff --git a/tracetool b/tracetool >> index 534cc70..c7582bf 100755 >> --- a/tracetool >> +++ b/tracetool >> @@ -48,7 +48,8 @@ get_argnames() >> { >> local nfields field name >> nfields=0 >> - for field in $(get_args "$1"); do >> + args=get_args "$1" >> + for field in "$args"; do > > This part yes. (I took the liberty of adding args to the local line above) > >> nfields=$((nfields + 1)) > > Next error is here: > ./tracetool: syntax error at line 53: `nfields=$' unexpected That looks like fully standards compliant, so Solaris' /bin/sh is not. Can you try what happens with /usr/xpg4/bin/sh?
Am 12.09.2010 um 23:05 schrieb Blue Swirl: > On Sun, Sep 12, 2010 at 5:58 PM, Andreas Färber <andreas.faerber@web.de > > wrote: >> Am 12.09.2010 um 19:47 schrieb Blue Swirl: >>> nfields=$((nfields + 1)) >> >> ./tracetool: syntax error at line 53: `nfields=$' unexpected > > That looks like fully standards compliant, so Solaris' /bin/sh is not. > Can you try what happens with /usr/xpg4/bin/sh? Works fine! Must've done something wrong when testing that earlier today. configure, create_config and tracetool with your fix are silent when / usr/xpg4/bin is in the $PATH. If you commit it, we can close this ticket. Thanks for your help, Blue. Build still fails, in qemu-nbd.c due to err.h, but that's unrelated to /bin/sh. After disabling the tools in configure, sparc-softmmu builds fine again.
13.09.2010 01:05, Blue Swirl wrote: > On Sun, Sep 12, 2010 at 5:58 PM, Andreas Färber <andreas.faerber@web.de> wrote: >> Am 12.09.2010 um 19:47 schrieb Blue Swirl: >> >>> On Sun, Sep 12, 2010 at 5:35 PM, Andreas Färber <andreas.faerber@web.de> >>> wrote: >>>> >>>> Am 12.09.2010 um 19:22 schrieb Blue Swirl: >>>> >>>>> What is the output of "sh ./tracetool --nop --check-backend"? >>>> >>>> ./tracetool: syntax error at line 51: `$' unexpected >>> >>> Does this patch fix the problem? >>> >>> diff --git a/tracetool b/tracetool >>> index 534cc70..c7582bf 100755 >>> --- a/tracetool >>> +++ b/tracetool >>> @@ -48,7 +48,8 @@ get_argnames() >>> { >>> local nfields field name >>> nfields=0 >>> - for field in $(get_args "$1"); do >>> + args=get_args "$1" >>> + for field in "$args"; do >> >> This part yes. (I took the liberty of adding args to the local line above) Um. Are you sure it works as expected? I'm not at all shure. There are 2 errors in the above patch: + args=get_args "$1" After this line, variable $args will contain one word: "get_args". Shell will try to execute a command or call a shell function which name is stored in $1, if it is assigned. If it is not, at least bash will complain that it can't execute command "". The proper way is to add backticks: + args=`get_args "$1"` In the second line: + for field in "$args"; do the double quotes ensure that all words in $args are processed as single word, all at once. So the for loop will be executed exactly one time, no matter how many arguments are given (even if there's none). So the right solution is to drop double quotes. JFYI. /mjt
Am 13.09.2010 um 10:49 schrieb Michael Tokarev: > 13.09.2010 01:05, Blue Swirl wrote: >> On Sun, Sep 12, 2010 at 5:58 PM, Andreas Färber <andreas.faerber@web.de >> > wrote: >>> Am 12.09.2010 um 19:47 schrieb Blue Swirl: >>> >>>> diff --git a/tracetool b/tracetool >>>> index 534cc70..c7582bf 100755 >>>> --- a/tracetool >>>> +++ b/tracetool >>>> @@ -48,7 +48,8 @@ get_argnames() >>>> { >>>> local nfields field name >>>> nfields=0 >>>> - for field in $(get_args "$1"); do >>>> + args=get_args "$1" >>>> + for field in "$args"; do >>> >>> This part yes. (I took the liberty of adding args to the local >>> line above) > > Um. Are you sure it works as expected? No, I'm not sure. It's Dark Magic to me and happened to unbreak configure; I'll try out your suggestions the next days. Thanks, Andreas > I'm not at all shure. > There are 2 errors in the above patch: > > + args=get_args "$1" > > After this line, variable $args will contain one word: "get_args". > Shell will try to execute a command or call a shell function which > name is stored in $1, if it is assigned. If it is not, at least > bash will complain that it can't execute command "". > > The proper way is to add backticks: > > + args=`get_args "$1"` > > In the second line: > > + for field in "$args"; do > > the double quotes ensure that all words in $args are > processed as single word, all at once. So the for loop > will be executed exactly one time, no matter how many > arguments are given (even if there's none). > > So the right solution is to drop double quotes. > > JFYI. > > /mjt >
On Sun, Sep 12, 2010 at 10:02 PM, Andreas Färber <andreas.faerber@web.de> wrote: > Am 12.09.2010 um 23:05 schrieb Blue Swirl: > >> On Sun, Sep 12, 2010 at 5:58 PM, Andreas Färber <andreas.faerber@web.de> >> wrote: >>> >>> Am 12.09.2010 um 19:47 schrieb Blue Swirl: >>>> >>>> nfields=$((nfields + 1)) >>> >>> ./tracetool: syntax error at line 53: `nfields=$' unexpected >> >> That looks like fully standards compliant, so Solaris' /bin/sh is not. >> Can you try what happens with /usr/xpg4/bin/sh? > > Works fine! Must've done something wrong when testing that earlier today. > > configure, create_config and tracetool with your fix are silent when > /usr/xpg4/bin is in the $PATH. > If you commit it, we can close this ticket. Thanks for your help, Blue. Does /usr/xpg4/bin/sh work without the patch?
On Mon, Sep 13, 2010 at 8:49 AM, Michael Tokarev <mjt@tls.msk.ru> wrote: > 13.09.2010 01:05, Blue Swirl wrote: >> On Sun, Sep 12, 2010 at 5:58 PM, Andreas Färber <andreas.faerber@web.de> wrote: >>> Am 12.09.2010 um 19:47 schrieb Blue Swirl: >>> >>>> On Sun, Sep 12, 2010 at 5:35 PM, Andreas Färber <andreas.faerber@web.de> >>>> wrote: >>>>> >>>>> Am 12.09.2010 um 19:22 schrieb Blue Swirl: >>>>> >>>>>> What is the output of "sh ./tracetool --nop --check-backend"? >>>>> >>>>> ./tracetool: syntax error at line 51: `$' unexpected >>>> >>>> Does this patch fix the problem? >>>> >>>> diff --git a/tracetool b/tracetool >>>> index 534cc70..c7582bf 100755 >>>> --- a/tracetool >>>> +++ b/tracetool >>>> @@ -48,7 +48,8 @@ get_argnames() >>>> { >>>> local nfields field name >>>> nfields=0 >>>> - for field in $(get_args "$1"); do >>>> + args=get_args "$1" >>>> + for field in "$args"; do >>> >>> This part yes. (I took the liberty of adding args to the local line above) > > Um. Are you sure it works as expected? I'm not at all shure. > There are 2 errors in the above patch: > > + args=get_args "$1" > > After this line, variable $args will contain one word: "get_args". > Shell will try to execute a command or call a shell function which > name is stored in $1, if it is assigned. If it is not, at least > bash will complain that it can't execute command "". > > The proper way is to add backticks: > > + args=`get_args "$1"` > > In the second line: > > + for field in "$args"; do > > the double quotes ensure that all words in $args are > processed as single word, all at once. So the for loop > will be executed exactly one time, no matter how many > arguments are given (even if there's none). > > So the right solution is to drop double quotes. Do you see any bug with the original? If the problem is in fact that Solaris' /bin/sh is not standards compliant, we shouldn't fix the script but instead make sure that the shell used to run tracetool is the compliant one.
14.09.2010 20:41, blueswirl wrote: > On Mon, Sep 13, 2010 at 8:49 AM, Michael Tokarev <mjt@tls.msk.ru> wrote: [] >>>>> diff --git a/tracetool b/tracetool >>>>> index 534cc70..c7582bf 100755 >>>>> --- a/tracetool >>>>> +++ b/tracetool >>>>> @@ -48,7 +48,8 @@ get_argnames() >>>>> { >>>>> local nfields field name >>>>> nfields=0 >>>>> - for field in $(get_args "$1"); do >>>>> + args=get_args "$1" >>>>> + for field in "$args"; do >>>> >>>> This part yes. (I took the liberty of adding args to the local line above) >> >> Um. Are you sure it works as expected? I'm not at all shure. >> There are 2 errors in the above patch: [] > Do you see any bug with the original? The thing is - I've no idea what's the talk about. There's no mentions of tracetool in 0.13.0-rc1 tarball. > If the problem is in fact that Solaris' /bin/sh is not standards > compliant, we shouldn't fix the script but instead make sure that the > shell used to run tracetool is the compliant one. Solaris's /bin/sh isn't exactly non-standard-compliant, it's very limited. In particular, $() construct weren't in the standard for quite some time, and I'm not sure it is now. The usual idiom is to use backticks `` for this. Only in case of nested subshells or using case..esac (with closing brases for individual cases) standard backticks does not work. /mjt
Am 14.09.2010 um 18:34 schrieb Blue Swirl: > On Sun, Sep 12, 2010 at 10:02 PM, Andreas Färber <andreas.faerber@web.de > > wrote: >> Am 12.09.2010 um 23:05 schrieb Blue Swirl: >> >>> On Sun, Sep 12, 2010 at 5:58 PM, Andreas Färber <andreas.faerber@web.de >>> > >>> wrote: >>>> >>>> Am 12.09.2010 um 19:47 schrieb Blue Swirl: >>>>> >>>>> nfields=$((nfields + 1)) >>>> >>>> ./tracetool: syntax error at line 53: `nfields=$' unexpected >>> >>> That looks like fully standards compliant, so Solaris' /bin/sh is >>> not. >>> Can you try what happens with /usr/xpg4/bin/sh? >> >> Works fine! Must've done something wrong when testing that earlier >> today. >> >> configure, create_config and tracetool with your fix are silent when >> /usr/xpg4/bin is in the $PATH. >> If you commit it, we can close this ticket. Thanks for your help, >> Blue. > > Does /usr/xpg4/bin/sh work without the patch? No.
On Tue, Sep 14, 2010 at 8:37 PM, Andreas Färber <andreas.faerber@web.de> wrote: > Am 14.09.2010 um 18:34 schrieb Blue Swirl: > >> On Sun, Sep 12, 2010 at 10:02 PM, Andreas Färber <andreas.faerber@web.de> >> wrote: >>> >>> Am 12.09.2010 um 23:05 schrieb Blue Swirl: >>> >>>> On Sun, Sep 12, 2010 at 5:58 PM, Andreas Färber <andreas.faerber@web.de> >>>> wrote: >>>>> >>>>> Am 12.09.2010 um 19:47 schrieb Blue Swirl: >>>>>> >>>>>> nfields=$((nfields + 1)) >>>>> >>>>> ./tracetool: syntax error at line 53: `nfields=$' unexpected >>>> >>>> That looks like fully standards compliant, so Solaris' /bin/sh is not. >>>> Can you try what happens with /usr/xpg4/bin/sh? >>> >>> Works fine! Must've done something wrong when testing that earlier today. >>> >>> configure, create_config and tracetool with your fix are silent when >>> /usr/xpg4/bin is in the $PATH. >>> If you commit it, we can close this ticket. Thanks for your help, Blue. >> >> Does /usr/xpg4/bin/sh work without the patch? > > No. How about with the attached patch? If yes, does it work even with /bin/sh?
Am 14.09.2010 um 22:53 schrieb Blue Swirl: > How about with the attached patch? If yes, does it work even with / > bin/sh? LC_ALL=C /usr/xpg4/bin/sh ./tracetool --nop --check-backend works fine, LC_ALL=C sh ./tracetool --nop --check-backend ./tracetool: bad substitution
diff --git a/tracetool b/tracetool index 534cc70..c7582bf 100755 --- a/tracetool +++ b/tracetool @@ -48,7 +48,8 @@ get_argnames() { local nfields field name nfields=0 - for field in $(get_args "$1"); do + args=get_args "$1" + for field in "$args"; do nfields=$((nfields + 1)) # Drop pointer star