| Message ID | 20200821150014.42461-1-sgarzare@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | configure: silence 'shift' error message in version_ge() | expand |
On 21/08/20 17:00, Stefano Garzarella wrote: > If there are less than 2 arguments in version_ge(), the second shift > prints this error: > ../configure: line 232: shift: shift count out of range > > Let's shut it up, since we're expecting this situation. > > Signed-off-by: Stefano Garzarella <sgarzare@redhat.com> > --- > configure | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/configure b/configure > index 4e5fe33211..de4bd0df36 100755 > --- a/configure > +++ b/configure > @@ -229,7 +229,7 @@ version_ge () { > set x $local_ver1 > local_first=${2-0} > # shift 2 does nothing if there are less than 2 arguments > - shift; shift > + shift; shift 2>/dev/null > local_ver1=$* > set x $local_ver2 > # the second argument finished, the first must be greater or equal > Acked-by: Paolo Bonzini <pbonzini@redhat.com>
On Fri, 21 Aug 2020 at 16:00, Stefano Garzarella <sgarzare@redhat.com> wrote: > > If there are less than 2 arguments in version_ge(), the second shift > prints this error: > ../configure: line 232: shift: shift count out of range > > Let's shut it up, since we're expecting this situation. > > Signed-off-by: Stefano Garzarella <sgarzare@redhat.com> > --- > configure | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/configure b/configure > index 4e5fe33211..de4bd0df36 100755 > --- a/configure > +++ b/configure > @@ -229,7 +229,7 @@ version_ge () { > set x $local_ver1 > local_first=${2-0} > # shift 2 does nothing if there are less than 2 arguments > - shift; shift > + shift; shift 2>/dev/null POSIX says https://pubs.opengroup.org/onlinepubs/9699919799/utilities/V3_chap02.html#shift "If the n operand is invalid or is greater than "$#", this may be considered a syntax error and a non-interactive shell may exit" so I think that we need to actually avoid the excess shift, not just suppress any warning it might print. (I'm not sure Philippe's "shift || shift" patch can work for that, though, as the exit status doesn't distinguish "valid shift but don't do it again" from "valid shift and more args to come".) thanks -- PMM
On Fri, Aug 21, 2020 at 04:21:10PM +0100, Peter Maydell wrote: > On Fri, 21 Aug 2020 at 16:00, Stefano Garzarella <sgarzare@redhat.com> wrote: > > > > If there are less than 2 arguments in version_ge(), the second shift > > prints this error: > > ../configure: line 232: shift: shift count out of range > > > > Let's shut it up, since we're expecting this situation. > > > > Signed-off-by: Stefano Garzarella <sgarzare@redhat.com> > > --- > > configure | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/configure b/configure > > index 4e5fe33211..de4bd0df36 100755 > > --- a/configure > > +++ b/configure > > @@ -229,7 +229,7 @@ version_ge () { > > set x $local_ver1 > > local_first=${2-0} > > # shift 2 does nothing if there are less than 2 arguments > > - shift; shift > > + shift; shift 2>/dev/null > > POSIX says > https://pubs.opengroup.org/onlinepubs/9699919799/utilities/V3_chap02.html#shift > > "If the n operand is invalid or is greater than "$#", this may be > considered a syntax error and a non-interactive shell may exit" > > so I think that we need to actually avoid the excess shift, Maybe something like this: diff --git a/configure b/configure index de4bd0df36..5f5f370e2c 100755 --- a/configure +++ b/configure @@ -229,7 +229,7 @@ version_ge () { set x $local_ver1 local_first=${2-0} # shift 2 does nothing if there are less than 2 arguments - shift; shift + shift; test $# -gt 0 && shift local_ver1=$* set x $local_ver2 # the second argument finished, the first must be greater or equal > not just suppress any warning it might print. (I'm not sure > Philippe's "shift || shift" patch can work for that, though, > as the exit status doesn't distinguish "valid shift but don't > do it again" from "valid shift and more args to come".) I tried and also if I have meson 0.55.0, with the Philippe's patch applied it tries to download our internal meson, so maybe it is not working as expected. Thanks, Stefano
On 8/21/20 5:26 PM, Stefano Garzarella wrote: > On Fri, Aug 21, 2020 at 04:21:10PM +0100, Peter Maydell wrote: >> On Fri, 21 Aug 2020 at 16:00, Stefano Garzarella <sgarzare@redhat.com> wrote: >>> >>> If there are less than 2 arguments in version_ge(), the second shift >>> prints this error: >>> ../configure: line 232: shift: shift count out of range >>> >>> Let's shut it up, since we're expecting this situation. Maybe s/shut up/silence/? >>> >>> Signed-off-by: Stefano Garzarella <sgarzare@redhat.com> >>> --- >>> configure | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/configure b/configure >>> index 4e5fe33211..de4bd0df36 100755 >>> --- a/configure >>> +++ b/configure >>> @@ -229,7 +229,7 @@ version_ge () { >>> set x $local_ver1 >>> local_first=${2-0} >>> # shift 2 does nothing if there are less than 2 arguments >>> - shift; shift >>> + shift; shift 2>/dev/null >> >> POSIX says >> https://pubs.opengroup.org/onlinepubs/9699919799/utilities/V3_chap02.html#shift >> >> "If the n operand is invalid or is greater than "$#", this may be >> considered a syntax error and a non-interactive shell may exit" >> >> so I think that we need to actually avoid the excess shift, > > Maybe something like this: > > diff --git a/configure b/configure > index de4bd0df36..5f5f370e2c 100755 > --- a/configure > +++ b/configure > @@ -229,7 +229,7 @@ version_ge () { > set x $local_ver1 > local_first=${2-0} > # shift 2 does nothing if there are less than 2 arguments > - shift; shift > + shift; test $# -gt 0 && shift This looks better that mine indeed. > local_ver1=$* > set x $local_ver2 > # the second argument finished, the first must be greater or equal > >> not just suppress any warning it might print. (I'm not sure >> Philippe's "shift || shift" patch can work for that, though, >> as the exit status doesn't distinguish "valid shift but don't >> do it again" from "valid shift and more args to come".) > > I tried and also if I have meson 0.55.0, with the Philippe's patch > applied it tries to download our internal meson, so maybe it is not > working as expected. =) > > Thanks, > Stefano >
On Fri, Aug 21, 2020 at 06:14:25PM +0200, Philippe Mathieu-Daudé wrote: > On 8/21/20 5:26 PM, Stefano Garzarella wrote: > > On Fri, Aug 21, 2020 at 04:21:10PM +0100, Peter Maydell wrote: > >> On Fri, 21 Aug 2020 at 16:00, Stefano Garzarella <sgarzare@redhat.com> wrote: > >>> > >>> If there are less than 2 arguments in version_ge(), the second shift > >>> prints this error: > >>> ../configure: line 232: shift: shift count out of range > >>> > >>> Let's shut it up, since we're expecting this situation. > > Maybe s/shut up/silence/? Yeah, less aggressive ;-) > > >>> > >>> Signed-off-by: Stefano Garzarella <sgarzare@redhat.com> > >>> --- > >>> configure | 2 +- > >>> 1 file changed, 1 insertion(+), 1 deletion(-) > >>> > >>> diff --git a/configure b/configure > >>> index 4e5fe33211..de4bd0df36 100755 > >>> --- a/configure > >>> +++ b/configure > >>> @@ -229,7 +229,7 @@ version_ge () { > >>> set x $local_ver1 > >>> local_first=${2-0} > >>> # shift 2 does nothing if there are less than 2 arguments > >>> - shift; shift > >>> + shift; shift 2>/dev/null > >> > >> POSIX says > >> https://pubs.opengroup.org/onlinepubs/9699919799/utilities/V3_chap02.html#shift > >> > >> "If the n operand is invalid or is greater than "$#", this may be > >> considered a syntax error and a non-interactive shell may exit" > >> > >> so I think that we need to actually avoid the excess shift, > > > > Maybe something like this: > > > > diff --git a/configure b/configure > > index de4bd0df36..5f5f370e2c 100755 > > --- a/configure > > +++ b/configure > > @@ -229,7 +229,7 @@ version_ge () { > > set x $local_ver1 > > local_first=${2-0} > > # shift 2 does nothing if there are less than 2 arguments > > - shift; shift > > + shift; test $# -gt 0 && shift > > This looks better that mine indeed. Okay, I'll send a v2 with this change. Thanks, Stefano > > > local_ver1=$* > > set x $local_ver2 > > # the second argument finished, the first must be greater or equal > > > >> not just suppress any warning it might print. (I'm not sure > >> Philippe's "shift || shift" patch can work for that, though, > >> as the exit status doesn't distinguish "valid shift but don't > >> do it again" from "valid shift and more args to come".) > > > > I tried and also if I have meson 0.55.0, with the Philippe's patch > > applied it tries to download our internal meson, so maybe it is not > > working as expected. > > =) > > > > > Thanks, > > Stefano > > >
On 8/21/20 10:21 AM, Peter Maydell wrote: > On Fri, 21 Aug 2020 at 16:00, Stefano Garzarella <sgarzare@redhat.com> wrote: >> >> If there are less than 2 arguments in version_ge(), the second shift >> prints this error: >> ../configure: line 232: shift: shift count out of range >> >> Let's shut it up, since we're expecting this situation. >> >> Signed-off-by: Stefano Garzarella <sgarzare@redhat.com> >> --- >> configure | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/configure b/configure >> index 4e5fe33211..de4bd0df36 100755 >> --- a/configure >> +++ b/configure >> @@ -229,7 +229,7 @@ version_ge () { >> set x $local_ver1 >> local_first=${2-0} >> # shift 2 does nothing if there are less than 2 arguments >> - shift; shift >> + shift; shift 2>/dev/null > > POSIX says > https://pubs.opengroup.org/onlinepubs/9699919799/utilities/V3_chap02.html#shift > > "If the n operand is invalid or is greater than "$#", this may be > considered a syntax error and a non-interactive shell may exit" > > so I think that we need to actually avoid the excess shift, > not just suppress any warning it might print. (I'm not sure > Philippe's "shift || shift" patch can work for that, though, > as the exit status doesn't distinguish "valid shift but don't > do it again" from "valid shift and more args to come".) Indeed. 'shift || shift' is not going to work. 'shift; shift 2>/dev/null' is risky, as it can cause the shell to exit. But this does exactly what you want: shift ${2:+2} which works out to 'shift 2' if $2 is set, or 'shift' (implicitly shift 1) if $2 is not set.
diff --git a/configure b/configure index 4e5fe33211..de4bd0df36 100755 --- a/configure +++ b/configure @@ -229,7 +229,7 @@ version_ge () { set x $local_ver1 local_first=${2-0} # shift 2 does nothing if there are less than 2 arguments - shift; shift + shift; shift 2>/dev/null local_ver1=$* set x $local_ver2 # the second argument finished, the first must be greater or equal
If there are less than 2 arguments in version_ge(), the second shift prints this error: ../configure: line 232: shift: shift count out of range Let's shut it up, since we're expecting this situation. Signed-off-by: Stefano Garzarella <sgarzare@redhat.com> --- configure | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)