| Message ID | mvmy1nfslvi.fsf@suse.de |
|---|---|
| State | New |
| Headers | show |
| Series | [v2] linux-user: preserve incoming order of environment variables in the target | expand |
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
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
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.
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 :| > > >
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
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.
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); }
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(+)