* [PATCH] elf: Properly remove the initial 'env' command
@ 2024-02-09 12:54 H.J. Lu
2024-02-09 13:00 ` Florian Weimer
2024-02-09 13:16 ` Andreas Schwab
0 siblings, 2 replies; 9+ messages in thread
From: H.J. Lu @ 2024-02-09 12:54 UTC (permalink / raw)
To: libc-alpha; +Cc: fweimer
tst-rtld-list-diagnostics.py is called by
"$(test-wrapper-env) $(objpfx)$(rtld-installed-name) --list-diagnostics"
and $(test-wrapper-env) is set to "$(test-wrapper) env". When there is
a test wrapper, it is incorrect to use:
# Remove the initial 'env' command.
parse_diagnostics(opts.command.split()[1:])
to remove 'env'. Change tst-rtld-list-diagnostics.py to explicitly check
'env' instead. This fixes [BZ #31357].
---
elf/tst-rtld-list-diagnostics.py | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/elf/tst-rtld-list-diagnostics.py b/elf/tst-rtld-list-diagnostics.py
index 9e70e74bf8..dfba94de64 100644
--- a/elf/tst-rtld-list-diagnostics.py
+++ b/elf/tst-rtld-list-diagnostics.py
@@ -294,7 +294,11 @@ def main(argv):
check_consistency_with_manual(opts.manual)
# Remove the initial 'env' command.
- parse_diagnostics(opts.command.split()[1:])
+ options = []
+ for o in opts.command.split()[0:]:
+ if o != 'env':
+ options.append(o)
+ parse_diagnostics(options)
if errors:
sys.exit(1)
--
2.43.0
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] elf: Properly remove the initial 'env' command
2024-02-09 12:54 [PATCH] elf: Properly remove the initial 'env' command H.J. Lu
@ 2024-02-09 13:00 ` Florian Weimer
2024-02-09 13:19 ` H.J. Lu
2024-02-09 13:16 ` Andreas Schwab
1 sibling, 1 reply; 9+ messages in thread
From: Florian Weimer @ 2024-02-09 13:00 UTC (permalink / raw)
To: H.J. Lu; +Cc: libc-alpha
* H. J. Lu:
> index 9e70e74bf8..dfba94de64 100644
> --- a/elf/tst-rtld-list-diagnostics.py
> +++ b/elf/tst-rtld-list-diagnostics.py
> @@ -294,7 +294,11 @@ def main(argv):
> check_consistency_with_manual(opts.manual)
>
> # Remove the initial 'env' command.
> - parse_diagnostics(opts.command.split()[1:])
> + options = []
> + for o in opts.command.split()[0:]:
> + if o != 'env':
> + options.append(o)
> + parse_diagnostics(options)
I think you can write this as:
options = opts.command.split()[:]
options.remove('env')
It only removes the first occurrence, but I think that is more correct
anyway.
Thanks,
Florian
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] elf: Properly remove the initial 'env' command
2024-02-09 12:54 [PATCH] elf: Properly remove the initial 'env' command H.J. Lu
2024-02-09 13:00 ` Florian Weimer
@ 2024-02-09 13:16 ` Andreas Schwab
2024-02-09 13:30 ` Florian Weimer
1 sibling, 1 reply; 9+ messages in thread
From: Andreas Schwab @ 2024-02-09 13:16 UTC (permalink / raw)
To: H.J. Lu; +Cc: libc-alpha, fweimer
On Feb 09 2024, H.J. Lu wrote:
> diff --git a/elf/tst-rtld-list-diagnostics.py b/elf/tst-rtld-list-diagnostics.py
> index 9e70e74bf8..dfba94de64 100644
> --- a/elf/tst-rtld-list-diagnostics.py
> +++ b/elf/tst-rtld-list-diagnostics.py
> @@ -294,7 +294,11 @@ def main(argv):
> check_consistency_with_manual(opts.manual)
>
> # Remove the initial 'env' command.
> - parse_diagnostics(opts.command.split()[1:])
> + options = []
> + for o in opts.command.split()[0:]:
> + if o != 'env':
> + options.append(o)
Why does it need to do that in the first place?
--
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510 2552 DF73 E780 A9DA AEC1
"And now for something completely different."
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] elf: Properly remove the initial 'env' command
2024-02-09 13:00 ` Florian Weimer
@ 2024-02-09 13:19 ` H.J. Lu
0 siblings, 0 replies; 9+ messages in thread
From: H.J. Lu @ 2024-02-09 13:19 UTC (permalink / raw)
To: Florian Weimer; +Cc: libc-alpha
On Fri, Feb 9, 2024 at 5:00 AM Florian Weimer <fweimer@redhat.com> wrote:
>
> * H. J. Lu:
>
> > index 9e70e74bf8..dfba94de64 100644
> > --- a/elf/tst-rtld-list-diagnostics.py
> > +++ b/elf/tst-rtld-list-diagnostics.py
> > @@ -294,7 +294,11 @@ def main(argv):
> > check_consistency_with_manual(opts.manual)
> >
> > # Remove the initial 'env' command.
> > - parse_diagnostics(opts.command.split()[1:])
> > + options = []
> > + for o in opts.command.split()[0:]:
> > + if o != 'env':
> > + options.append(o)
> > + parse_diagnostics(options)
>
> I think you can write this as:
>
> options = opts.command.split()[:]
> options.remove('env')
>
> It only removes the first occurrence, but I think that is more correct
> anyway.
>
Fixed in v2.
Thanks.
--
H.J.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] elf: Properly remove the initial 'env' command
2024-02-09 13:16 ` Andreas Schwab
@ 2024-02-09 13:30 ` Florian Weimer
2024-02-09 13:52 ` H.J. Lu
0 siblings, 1 reply; 9+ messages in thread
From: Florian Weimer @ 2024-02-09 13:30 UTC (permalink / raw)
To: Andreas Schwab; +Cc: H.J. Lu, libc-alpha
* Andreas Schwab:
> On Feb 09 2024, H.J. Lu wrote:
>
>> diff --git a/elf/tst-rtld-list-diagnostics.py b/elf/tst-rtld-list-diagnostics.py
>> index 9e70e74bf8..dfba94de64 100644
>> --- a/elf/tst-rtld-list-diagnostics.py
>> +++ b/elf/tst-rtld-list-diagnostics.py
>> @@ -294,7 +294,11 @@ def main(argv):
>> check_consistency_with_manual(opts.manual)
>>
>> # Remove the initial 'env' command.
>> - parse_diagnostics(opts.command.split()[1:])
>> + options = []
>> + for o in opts.command.split()[0:]:
>> + if o != 'env':
>> + options.append(o)
>
> Why does it need to do that in the first place?
Good question. I must have copied it from scripts/tst-ld-trace.py. I
think we can remove the .split() and run the command string with
shell=True.
Thanks,
Florian
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] elf: Properly remove the initial 'env' command
2024-02-09 13:30 ` Florian Weimer
@ 2024-02-09 13:52 ` H.J. Lu
2024-02-09 14:53 ` Florian Weimer
0 siblings, 1 reply; 9+ messages in thread
From: H.J. Lu @ 2024-02-09 13:52 UTC (permalink / raw)
To: Florian Weimer; +Cc: Andreas Schwab, libc-alpha
On Fri, Feb 9, 2024 at 5:30 AM Florian Weimer <fweimer@redhat.com> wrote:
>
> * Andreas Schwab:
>
> > On Feb 09 2024, H.J. Lu wrote:
> >
> >> diff --git a/elf/tst-rtld-list-diagnostics.py b/elf/tst-rtld-list-diagnostics.py
> >> index 9e70e74bf8..dfba94de64 100644
> >> --- a/elf/tst-rtld-list-diagnostics.py
> >> +++ b/elf/tst-rtld-list-diagnostics.py
> >> @@ -294,7 +294,11 @@ def main(argv):
> >> check_consistency_with_manual(opts.manual)
> >>
> >> # Remove the initial 'env' command.
> >> - parse_diagnostics(opts.command.split()[1:])
> >> + options = []
> >> + for o in opts.command.split()[0:]:
> >> + if o != 'env':
> >> + options.append(o)
> >
> > Why does it need to do that in the first place?
>
> Good question. I must have copied it from scripts/tst-ld-trace.py. I
> think we can remove the .split() and run the command string with
How does removing .split() work? The argument is
"/export/build/gnu/tools-build/glibc-apx-sde/test-sde.sh env
/export/build/gnu/tools-build/glibc-apx-sde/build-x86_64-linux/elf/ld-linux-x86-64.so.2
--list-diagnostics"
We need .split() to change it to proper arguments.
> shell=True.
>
> Thanks,
> Florian
>
--
H.J.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] elf: Properly remove the initial 'env' command
2024-02-09 13:52 ` H.J. Lu
@ 2024-02-09 14:53 ` Florian Weimer
2024-02-09 15:00 ` H.J. Lu
0 siblings, 1 reply; 9+ messages in thread
From: Florian Weimer @ 2024-02-09 14:53 UTC (permalink / raw)
To: H.J. Lu; +Cc: Andreas Schwab, libc-alpha
* H. J. Lu:
> On Fri, Feb 9, 2024 at 5:30 AM Florian Weimer <fweimer@redhat.com> wrote:
>>
>> * Andreas Schwab:
>>
>> > On Feb 09 2024, H.J. Lu wrote:
>> >
>> >> diff --git a/elf/tst-rtld-list-diagnostics.py b/elf/tst-rtld-list-diagnostics.py
>> >> index 9e70e74bf8..dfba94de64 100644
>> >> --- a/elf/tst-rtld-list-diagnostics.py
>> >> +++ b/elf/tst-rtld-list-diagnostics.py
>> >> @@ -294,7 +294,11 @@ def main(argv):
>> >> check_consistency_with_manual(opts.manual)
>> >>
>> >> # Remove the initial 'env' command.
>> >> - parse_diagnostics(opts.command.split()[1:])
>> >> + options = []
>> >> + for o in opts.command.split()[0:]:
>> >> + if o != 'env':
>> >> + options.append(o)
>> >
>> > Why does it need to do that in the first place?
>>
>> Good question. I must have copied it from scripts/tst-ld-trace.py. I
>> think we can remove the .split() and run the command string with
>
> How does removing .split() work? The argument is
>
> "/export/build/gnu/tools-build/glibc-apx-sde/test-sde.sh env
> /export/build/gnu/tools-build/glibc-apx-sde/build-x86_64-linux/elf/ld-linux-x86-64.so.2
> --list-diagnostics"
>
> We need .split() to change it to proper arguments.
Can you test this?
diff --git a/elf/tst-rtld-list-diagnostics.py b/elf/tst-rtld-list-diagnostics.py
index 9e70e74bf8..024bd8c320 100644
--- a/elf/tst-rtld-list-diagnostics.py
+++ b/elf/tst-rtld-list-diagnostics.py
@@ -222,7 +222,7 @@ else:
def parse_diagnostics(cmd):
global errors
diag_out = subprocess.run(cmd, stdout=subprocess.PIPE, check=True,
- universal_newlines=True).stdout
+ universal_newlines=True, shell=True).stdout
if diag_out[-1] != '\n':
print('error: ld.so output does not end in newline')
errors += 1
@@ -293,8 +293,7 @@ def main(argv):
if opts.manual:
check_consistency_with_manual(opts.manual)
- # Remove the initial 'env' command.
- parse_diagnostics(opts.command.split()[1:])
+ parse_diagnostics(opts.command)
if errors:
sys.exit(1)
Thanks,
Florian
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] elf: Properly remove the initial 'env' command
2024-02-09 14:53 ` Florian Weimer
@ 2024-02-09 15:00 ` H.J. Lu
2024-02-09 15:17 ` Florian Weimer
0 siblings, 1 reply; 9+ messages in thread
From: H.J. Lu @ 2024-02-09 15:00 UTC (permalink / raw)
To: Florian Weimer; +Cc: Andreas Schwab, libc-alpha
On Fri, Feb 9, 2024 at 6:53 AM Florian Weimer <fweimer@redhat.com> wrote:
>
> * H. J. Lu:
>
> > On Fri, Feb 9, 2024 at 5:30 AM Florian Weimer <fweimer@redhat.com> wrote:
> >>
> >> * Andreas Schwab:
> >>
> >> > On Feb 09 2024, H.J. Lu wrote:
> >> >
> >> >> diff --git a/elf/tst-rtld-list-diagnostics.py b/elf/tst-rtld-list-diagnostics.py
> >> >> index 9e70e74bf8..dfba94de64 100644
> >> >> --- a/elf/tst-rtld-list-diagnostics.py
> >> >> +++ b/elf/tst-rtld-list-diagnostics.py
> >> >> @@ -294,7 +294,11 @@ def main(argv):
> >> >> check_consistency_with_manual(opts.manual)
> >> >>
> >> >> # Remove the initial 'env' command.
> >> >> - parse_diagnostics(opts.command.split()[1:])
> >> >> + options = []
> >> >> + for o in opts.command.split()[0:]:
> >> >> + if o != 'env':
> >> >> + options.append(o)
> >> >
> >> > Why does it need to do that in the first place?
> >>
> >> Good question. I must have copied it from scripts/tst-ld-trace.py. I
> >> think we can remove the .split() and run the command string with
> >
> > How does removing .split() work? The argument is
> >
> > "/export/build/gnu/tools-build/glibc-apx-sde/test-sde.sh env
> > /export/build/gnu/tools-build/glibc-apx-sde/build-x86_64-linux/elf/ld-linux-x86-64.so.2
> > --list-diagnostics"
> >
> > We need .split() to change it to proper arguments.
>
> Can you test this?
>
> diff --git a/elf/tst-rtld-list-diagnostics.py b/elf/tst-rtld-list-diagnostics.py
> index 9e70e74bf8..024bd8c320 100644
> --- a/elf/tst-rtld-list-diagnostics.py
> +++ b/elf/tst-rtld-list-diagnostics.py
> @@ -222,7 +222,7 @@ else:
> def parse_diagnostics(cmd):
> global errors
> diag_out = subprocess.run(cmd, stdout=subprocess.PIPE, check=True,
> - universal_newlines=True).stdout
> + universal_newlines=True, shell=True).stdout
> if diag_out[-1] != '\n':
> print('error: ld.so output does not end in newline')
> errors += 1
> @@ -293,8 +293,7 @@ def main(argv):
> if opts.manual:
> check_consistency_with_manual(opts.manual)
>
> - # Remove the initial 'env' command.
> - parse_diagnostics(opts.command.split()[1:])
> + parse_diagnostics(opts.command)
>
> if errors:
> sys.exit(1)
>
> Thanks,
> Florian
>
It works. Can you check it in?
Thanks.
--
H.J.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] elf: Properly remove the initial 'env' command
2024-02-09 15:00 ` H.J. Lu
@ 2024-02-09 15:17 ` Florian Weimer
0 siblings, 0 replies; 9+ messages in thread
From: Florian Weimer @ 2024-02-09 15:17 UTC (permalink / raw)
To: H.J. Lu; +Cc: Andreas Schwab, libc-alpha
* H. J. Lu:
> It works. Can you check it in?
Thanks for checking, pushed.
Florian
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2024-02-09 15:17 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-02-09 12:54 [PATCH] elf: Properly remove the initial 'env' command H.J. Lu
2024-02-09 13:00 ` Florian Weimer
2024-02-09 13:19 ` H.J. Lu
2024-02-09 13:16 ` Andreas Schwab
2024-02-09 13:30 ` Florian Weimer
2024-02-09 13:52 ` H.J. Lu
2024-02-09 14:53 ` Florian Weimer
2024-02-09 15:00 ` H.J. Lu
2024-02-09 15:17 ` Florian Weimer
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).