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

Andreas Schwab posted 1 patch 1 year, 1 month ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/mvmy1nfslvi.fsf@suse.de
Maintainers: Laurent Vivier <laurent@vivier.eu>
linux-user/main.c | 6 ++++++
1 file changed, 6 insertions(+)
[PATCH v2] linux-user: preserve incoming order of environment variables in the target
Posted by Andreas Schwab 1 year, 1 month ago
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
Posted by Daniel P. Berrangé 1 year, 1 month ago
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
Posted by Warner Losh 1 year, 1 month ago
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 :|
>
>
>
Re: [PATCH v2] linux-user: preserve incoming order of environment variables in the target
Posted by Andreas Schwab 1 year, 1 month ago
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
Posted by Daniel P. Berrangé 1 year, 1 month ago
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
Posted by Philippe Mathieu-Daudé 1 year, 1 month ago
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
Posted by Andreas Schwab 1 year, 1 month ago
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."