1. Patchew
  2. QEMU
  3. View series

[PATCH] configure: silence 'shift' error message in version_ge()

Stefano Garzarella posted 1 patch 5 years, 5 months ago
Test docker-mingw@fedora failed
Test docker-quick@centos7 failed
Test checkpatch passed
Test FreeBSD failed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20200821150014.42461-1-sgarzare@redhat.com
There is a newer version of this series
configure | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
[PATCH] configure: silence 'shift' error message in version_ge()
Posted by Stefano Garzarella 5 years, 5 months ago 
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
-- 
2.26.2
Re: [PATCH] configure: silence 'shift' error message in version_ge()
Posted by Paolo Bonzini 5 years, 5 months ago 
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>
Re: [PATCH] configure: silence 'shift' error message in version_ge()
Posted by Peter Maydell 5 years, 5 months ago 
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
Re: [PATCH] configure: silence 'shift' error message in version_ge()
Posted by Stefano Garzarella 5 years, 5 months ago 
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
Re: [PATCH] configure: silence 'shift' error message in version_ge()
Posted by Philippe Mathieu-Daudé 5 years, 5 months ago 
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
>
Re: [PATCH] configure: silence 'shift' error message in version_ge()
Posted by Stefano Garzarella 5 years, 5 months ago 
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
> > 
>
Re: [PATCH] configure: silence 'shift' error message in version_ge()
Posted by Eric Blake 5 years, 5 months ago 
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.


-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.