[PATCH v2] linux-user: preserve incoming order of environment variables in the target

[PATCH v2] linux-user: preserve incoming order of environment variables in the target

[Andreas Schwab]

Do not reverse the order of environment variables in the target environ
array relative to the incoming environ order.  Some testsuites depend on a
specific order, even though it is not defined by any standard.

Signed-off-by: Andreas Schwab <schwab@suse.de>
---
 linux-user/main.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/linux-user/main.c b/linux-user/main.c
index 4b18461969..dbfd3ee8f1 100644
--- a/linux-user/main.c
+++ b/linux-user/main.c
@@ -691,7 +691,13 @@ int main(int argc, char **argv, char **envp)
     envlist = envlist_create();
 
     /* add current environment into the list */
+    /* envlist_setenv adds to the front of the list; to preserve environ
+       order add from back to front */
     for (wrk = environ; *wrk != NULL; wrk++) {
+        continue;
+    }
+    while (wrk != environ) {
+        wrk--;
         (void) envlist_setenv(envlist, *wrk);
     }
 
-- 
2.40.0


-- 
Andreas Schwab, SUSE Labs, schwab@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."

Re: [PATCH v2] linux-user: preserve incoming order of environment variables in the target

[Daniel P. Berrangé]

On Wed, Mar 29, 2023 at 03:55:13PM +0200, Andreas Schwab wrote:
> Do not reverse the order of environment variables in the target environ
> array relative to the incoming environ order.  Some testsuites depend on a
> specific order, even though it is not defined by any standard.
> 
> Signed-off-by: Andreas Schwab <schwab@suse.de>
> ---
>  linux-user/main.c | 6 ++++++
>  1 file changed, 6 insertions(+)

bsd-user/main.c appears to have an identical code pattern that
will need the same fix

> 
> diff --git a/linux-user/main.c b/linux-user/main.c
> index 4b18461969..dbfd3ee8f1 100644
> --- a/linux-user/main.c
> +++ b/linux-user/main.c
> @@ -691,7 +691,13 @@ int main(int argc, char **argv, char **envp)
>      envlist = envlist_create();
>  
>      /* add current environment into the list */
> +    /* envlist_setenv adds to the front of the list; to preserve environ
> +       order add from back to front */
>      for (wrk = environ; *wrk != NULL; wrk++) {
> +        continue;
> +    }
> +    while (wrk != environ) {
> +        wrk--;
>          (void) envlist_setenv(envlist, *wrk);
>      }
>  
> -- 
> 2.40.0
> 
> 
> -- 
> Andreas Schwab, SUSE Labs, schwab@suse.de
> GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
> "And now for something completely different."
> 

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [PATCH v2] linux-user: preserve incoming order of environment variables in the target

[Philippe Mathieu-Daudé]

On 29/3/23 16:00, Daniel P. Berrangé wrote:
> On Wed, Mar 29, 2023 at 03:55:13PM +0200, Andreas Schwab wrote:
>> Do not reverse the order of environment variables in the target environ
>> array relative to the incoming environ order.  Some testsuites depend on a
>> specific order, even though it is not defined by any standard.
>>
>> Signed-off-by: Andreas Schwab <schwab@suse.de>
>> ---
>>   linux-user/main.c | 6 ++++++
>>   1 file changed, 6 insertions(+)
> 
> bsd-user/main.c appears to have an identical code pattern that
> will need the same fix
> 
>>
>> diff --git a/linux-user/main.c b/linux-user/main.c
>> index 4b18461969..dbfd3ee8f1 100644
>> --- a/linux-user/main.c
>> +++ b/linux-user/main.c
>> @@ -691,7 +691,13 @@ int main(int argc, char **argv, char **envp)
>>       envlist = envlist_create();
>>   
>>       /* add current environment into the list */
>> +    /* envlist_setenv adds to the front of the list; to preserve environ
>> +       order add from back to front */

Also, QEMU coding style now requires:

   /*
    * this comment form.
    */

;)

>>       for (wrk = environ; *wrk != NULL; wrk++) {
>> +        continue;
>> +    }
>> +    while (wrk != environ) {
>> +        wrk--;
>>           (void) envlist_setenv(envlist, *wrk);
>>       }
>>   
>> -- 
>> 2.40.0
>>
>>
>> -- 
>> Andreas Schwab, SUSE Labs, schwab@suse.de
>> GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
>> "And now for something completely different."
>>
> 
> With regards,
> Daniel

Re: [PATCH v2] linux-user: preserve incoming order of environment variables in the target

[Andreas Schwab]

On Mär 29 2023, Philippe Mathieu-Daudé wrote:

> On 29/3/23 16:00, Daniel P. Berrangé wrote:
>> On Wed, Mar 29, 2023 at 03:55:13PM +0200, Andreas Schwab wrote:
>>> Do not reverse the order of environment variables in the target environ
>>> array relative to the incoming environ order.  Some testsuites depend on a
>>> specific order, even though it is not defined by any standard.
>>>
>>> Signed-off-by: Andreas Schwab <schwab@suse.de>
>>> ---
>>>   linux-user/main.c | 6 ++++++
>>>   1 file changed, 6 insertions(+)
>> bsd-user/main.c appears to have an identical code pattern that
>> will need the same fix
>> 
>>>
>>> diff --git a/linux-user/main.c b/linux-user/main.c
>>> index 4b18461969..dbfd3ee8f1 100644
>>> --- a/linux-user/main.c
>>> +++ b/linux-user/main.c
>>> @@ -691,7 +691,13 @@ int main(int argc, char **argv, char **envp)
>>>       envlist = envlist_create();
>>>         /* add current environment into the list */
>>> +    /* envlist_setenv adds to the front of the list; to preserve environ
>>> +       order add from back to front */
>
> Also, QEMU coding style now requires:
>
>   /*
>    * this comment form.
>    */

It's unfortunate that the next comment just below doesn't follow the
correct style, so I didn't notice.

-- 
Andreas Schwab, SUSE Labs, schwab@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."

Re: [PATCH v2] linux-user: preserve incoming order of environment variables in the target

[Andreas Schwab]

On Mär 29 2023, Daniel P. Berrangé wrote:

> On Wed, Mar 29, 2023 at 03:55:13PM +0200, Andreas Schwab wrote:
>> Do not reverse the order of environment variables in the target environ
>> array relative to the incoming environ order.  Some testsuites depend on a
>> specific order, even though it is not defined by any standard.
>> 
>> Signed-off-by: Andreas Schwab <schwab@suse.de>
>> ---
>>  linux-user/main.c | 6 ++++++
>>  1 file changed, 6 insertions(+)
>
> bsd-user/main.c appears to have an identical code pattern that
> will need the same fix

Yes, but I cannot test it, so I like to let someone else produce the
patch.

-- 
Andreas Schwab, SUSE Labs, schwab@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."

Re: [PATCH v2] linux-user: preserve incoming order of environment variables in the target

[Daniel P. Berrangé]

On Wed, Mar 29, 2023 at 04:04:43PM +0200, Andreas Schwab wrote:
> On Mär 29 2023, Daniel P. Berrangé wrote:
> 
> > On Wed, Mar 29, 2023 at 03:55:13PM +0200, Andreas Schwab wrote:
> >> Do not reverse the order of environment variables in the target environ
> >> array relative to the incoming environ order.  Some testsuites depend on a
> >> specific order, even though it is not defined by any standard.
> >> 
> >> Signed-off-by: Andreas Schwab <schwab@suse.de>
> >> ---
> >>  linux-user/main.c | 6 ++++++
> >>  1 file changed, 6 insertions(+)
> >
> > bsd-user/main.c appears to have an identical code pattern that
> > will need the same fix
> 
> Yes, but I cannot test it, so I like to let someone else produce the
> patch.

The code in this case is 100% identical, so I think it is
reasonable to expect that patch to cover both of them
regardless.

In terms of testing we don't require the contributors to test
all platform combinations affected. Our FreeBSD CI jobs will
test the build of bsd-user.

If so desired though, any contributor can easily test BSD changes
too via our VM infra. eg  "make vm-build-freebsd" (see make vm-help
for further options)

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [PATCH v2] linux-user: preserve incoming order of environment variables in the target

[Warner Losh]

[-- Attachment #1: Type: text/plain, Size: 1914 bytes --]

On Wed, Mar 29, 2023, 4:00 PM Daniel P. Berrangé <berrange@redhat.com>
wrote:

> On Wed, Mar 29, 2023 at 03:55:13PM +0200, Andreas Schwab wrote:
> > Do not reverse the order of environment variables in the target environ
> > array relative to the incoming environ order.  Some testsuites depend on
> a
> > specific order, even though it is not defined by any standard.
> >
> > Signed-off-by: Andreas Schwab <schwab@suse.de>
> > ---
> >  linux-user/main.c | 6 ++++++
> >  1 file changed, 6 insertions(+)
>
> bsd-user/main.c appears to have an identical code pattern that
> will need the same fix
>

Agreed. I am finishing up a vacation but was going to check on this... I
agree that bsd-user wants to do this too...

Warner

>
> > diff --git a/linux-user/main.c b/linux-user/main.c
> > index 4b18461969..dbfd3ee8f1 100644
> > --- a/linux-user/main.c
> > +++ b/linux-user/main.c
> > @@ -691,7 +691,13 @@ int main(int argc, char **argv, char **envp)
> >      envlist = envlist_create();
> >
> >      /* add current environment into the list */
> > +    /* envlist_setenv adds to the front of the list; to preserve environ
> > +       order add from back to front */
> >      for (wrk = environ; *wrk != NULL; wrk++) {
> > +        continue;
> > +    }
> > +    while (wrk != environ) {
> > +        wrk--;
> >          (void) envlist_setenv(envlist, *wrk);
> >      }
> >
> > --
> > 2.40.0
> >
> >
> > --
> > Andreas Schwab, SUSE Labs, schwab@suse.de
> > GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
> > "And now for something completely different."
> >
>
> With regards,
> Daniel
> --
> |: https://berrange.com      -o-
> https://www.flickr.com/photos/dberrange :|
> |: https://libvirt.org         -o-
> https://fstop138.berrange.com :|
> |: https://entangle-photo.org    -o-
> https://www.instagram.com/dberrange :|
>
>
>

[-- Attachment #2: Type: text/html, Size: 3428 bytes --]