* [LTP] [PATCH] process_state: Enhancement of process state detection
@ 2024-02-05 10:26 Li Wang
2024-02-06 11:51 ` Petr Vorel
0 siblings, 1 reply; 4+ messages in thread
From: Li Wang @ 2024-02-05 10:26 UTC (permalink / raw)
To: ltp
The functions will be more robust against process names with
unusual characters and will correctly read the state character
from the /proc/[pid]/stat file. This is a necessary change
because the process name, which is a free-form string, can
contain spaces and other characters that would otherwise
disrupt the simple parsing logic of the original format string.
e.g.
$ cat /proc/792442/stat
792442 (Web Content) S 164213 4351 4351 0 -1 4194560 ...
Reported-by: Ian Wienand <iwienand@redhat.com>
Signed-off-by: Li Wang <liwang@redhat.com>
Cc: Chunyu Hu <chuhu@redhat.com>
---
lib/tst_process_state.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/lib/tst_process_state.c b/lib/tst_process_state.c
index 08a9d0966..c15283c3d 100644
--- a/lib/tst_process_state.c
+++ b/lib/tst_process_state.c
@@ -22,7 +22,7 @@ int tst_process_state_wait(const char *file, const int lineno,
for (;;) {
safe_file_scanf(file, lineno, cleanup_fn, proc_path,
- "%*i %*s %c", &cur_state);
+ "%*[^)]%*c %c", &cur_state);
if (state == cur_state)
break;
@@ -54,7 +54,7 @@ int tst_process_state_wait2(pid_t pid, const char state)
return 1;
}
- if (fscanf(f, "%*i %*s %c", &cur_state) != 1) {
+ if (fscanf(f, "%*[^)]%*c %c", &cur_state) != 1) {
fclose(f);
fprintf(stderr, "Failed to read '%s': %s\n",
proc_path, strerror(errno));
--
2.40.1
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [LTP] [PATCH] process_state: Enhancement of process state detection
2024-02-05 10:26 [LTP] [PATCH] process_state: Enhancement of process state detection Li Wang
@ 2024-02-06 11:51 ` Petr Vorel
2024-02-06 12:59 ` Jan Stancek
2024-02-07 2:21 ` Li Wang
0 siblings, 2 replies; 4+ messages in thread
From: Petr Vorel @ 2024-02-06 11:51 UTC (permalink / raw)
To: Li Wang; +Cc: ltp
Hi Li, Ian,
> The functions will be more robust against process names with
> unusual characters and will correctly read the state character
> from the /proc/[pid]/stat file. This is a necessary change
> because the process name, which is a free-form string, can
> contain spaces and other characters that would otherwise
> disrupt the simple parsing logic of the original format string.
> e.g.
> $ cat /proc/792442/stat
> 792442 (Web Content) S 164213 4351 4351 0 -1 4194560 ...
> Reported-by: Ian Wienand <iwienand@redhat.com>
> Signed-off-by: Li Wang <liwang@redhat.com>
> Cc: Chunyu Hu <chuhu@redhat.com>
> ---
> lib/tst_process_state.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
> diff --git a/lib/tst_process_state.c b/lib/tst_process_state.c
> index 08a9d0966..c15283c3d 100644
> --- a/lib/tst_process_state.c
> +++ b/lib/tst_process_state.c
> @@ -22,7 +22,7 @@ int tst_process_state_wait(const char *file, const int lineno,
> for (;;) {
> safe_file_scanf(file, lineno, cleanup_fn, proc_path,
> - "%*i %*s %c", &cur_state);
> + "%*[^)]%*c %c", &cur_state);
Obviously correct, thanks.
Reviewed-by: Petr Vorel <pvorel@suse.cz>
But there is also the same issue in lib/tst_thread_state.c,
I guess it applies to that. Li, could you please also fix it before merge?
Also, Andrea, you added tst_thread_state_wait() and TST_THREAD_STATE_WAIT() for
futex_waitv0[23] related tests [1], but it's now not used anywhere due Jan's
changes [2] [3]. I wonder if it's still useful or whether we should remove it.
Kind regards,
Petr
[1] https://lore.kernel.org/ltp/20220209091756.17245-2-andrea.cervesato@suse.de/
[2] https://lore.kernel.org/ltp/6c5b161bc3bcf753cbda92954ca3f47cb268c68f.1663665637.git.jstancek@redhat.com/
[3] https://lore.kernel.org/ltp/6bac7035adc2cfc8ab3800fe1d2d03223ec57ff5.1663662348.git.jstancek@redhat.com/
> if (state == cur_state)
> break;
> @@ -54,7 +54,7 @@ int tst_process_state_wait2(pid_t pid, const char state)
> return 1;
> }
> - if (fscanf(f, "%*i %*s %c", &cur_state) != 1) {
> + if (fscanf(f, "%*[^)]%*c %c", &cur_state) != 1) {
> fclose(f);
> fprintf(stderr, "Failed to read '%s': %s\n",
> proc_path, strerror(errno));
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [LTP] [PATCH] process_state: Enhancement of process state detection
2024-02-06 11:51 ` Petr Vorel
@ 2024-02-06 12:59 ` Jan Stancek
2024-02-07 2:21 ` Li Wang
1 sibling, 0 replies; 4+ messages in thread
From: Jan Stancek @ 2024-02-06 12:59 UTC (permalink / raw)
To: Petr Vorel; +Cc: ltp
On Tue, Feb 6, 2024 at 12:51 PM Petr Vorel <pvorel@suse.cz> wrote:
>
> Hi Li, Ian,
>
> > The functions will be more robust against process names with
> > unusual characters and will correctly read the state character
> > from the /proc/[pid]/stat file. This is a necessary change
> > because the process name, which is a free-form string, can
> > contain spaces and other characters that would otherwise
> > disrupt the simple parsing logic of the original format string.
>
> > e.g.
> > $ cat /proc/792442/stat
> > 792442 (Web Content) S 164213 4351 4351 0 -1 4194560 ...
>
> > Reported-by: Ian Wienand <iwienand@redhat.com>
> > Signed-off-by: Li Wang <liwang@redhat.com>
> > Cc: Chunyu Hu <chuhu@redhat.com>
> > ---
> > lib/tst_process_state.c | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
>
> > diff --git a/lib/tst_process_state.c b/lib/tst_process_state.c
> > index 08a9d0966..c15283c3d 100644
> > --- a/lib/tst_process_state.c
> > +++ b/lib/tst_process_state.c
> > @@ -22,7 +22,7 @@ int tst_process_state_wait(const char *file, const int lineno,
>
> > for (;;) {
> > safe_file_scanf(file, lineno, cleanup_fn, proc_path,
> > - "%*i %*s %c", &cur_state);
> > + "%*[^)]%*c %c", &cur_state);
>
> Obviously correct, thanks.
>
> Reviewed-by: Petr Vorel <pvorel@suse.cz>
Acked-by: Jan Stancek <jstancek@redhat.com>
>
> But there is also the same issue in lib/tst_thread_state.c,
> I guess it applies to that. Li, could you please also fix it before merge?
>
> Also, Andrea, you added tst_thread_state_wait() and TST_THREAD_STATE_WAIT() for
> futex_waitv0[23] related tests [1], but it's now not used anywhere due Jan's
> changes [2] [3]. I wonder if it's still useful or whether we should remove it.
I'd keep it, it's a small function and someone could find useful for
some new test.
>
> Kind regards,
> Petr
>
> [1] https://lore.kernel.org/ltp/20220209091756.17245-2-andrea.cervesato@suse.de/
> [2] https://lore.kernel.org/ltp/6c5b161bc3bcf753cbda92954ca3f47cb268c68f.1663665637.git.jstancek@redhat.com/
> [3] https://lore.kernel.org/ltp/6bac7035adc2cfc8ab3800fe1d2d03223ec57ff5.1663662348.git.jstancek@redhat.com/
>
> > if (state == cur_state)
> > break;
> > @@ -54,7 +54,7 @@ int tst_process_state_wait2(pid_t pid, const char state)
> > return 1;
> > }
>
> > - if (fscanf(f, "%*i %*s %c", &cur_state) != 1) {
> > + if (fscanf(f, "%*[^)]%*c %c", &cur_state) != 1) {
> > fclose(f);
> > fprintf(stderr, "Failed to read '%s': %s\n",
> > proc_path, strerror(errno));
>
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [LTP] [PATCH] process_state: Enhancement of process state detection
2024-02-06 11:51 ` Petr Vorel
2024-02-06 12:59 ` Jan Stancek
@ 2024-02-07 2:21 ` Li Wang
1 sibling, 0 replies; 4+ messages in thread
From: Li Wang @ 2024-02-07 2:21 UTC (permalink / raw)
To: Petr Vorel; +Cc: ltp
On Tue, Feb 6, 2024 at 7:51 PM Petr Vorel <pvorel@suse.cz> wrote:
> Hi Li, Ian,
>
> > The functions will be more robust against process names with
> > unusual characters and will correctly read the state character
> > from the /proc/[pid]/stat file. This is a necessary change
> > because the process name, which is a free-form string, can
> > contain spaces and other characters that would otherwise
> > disrupt the simple parsing logic of the original format string.
>
> > e.g.
> > $ cat /proc/792442/stat
> > 792442 (Web Content) S 164213 4351 4351 0 -1 4194560 ...
>
> > Reported-by: Ian Wienand <iwienand@redhat.com>
> > Signed-off-by: Li Wang <liwang@redhat.com>
> > Cc: Chunyu Hu <chuhu@redhat.com>
> > ---
> > lib/tst_process_state.c | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
>
> > diff --git a/lib/tst_process_state.c b/lib/tst_process_state.c
> > index 08a9d0966..c15283c3d 100644
> > --- a/lib/tst_process_state.c
> > +++ b/lib/tst_process_state.c
> > @@ -22,7 +22,7 @@ int tst_process_state_wait(const char *file, const int
> lineno,
>
> > for (;;) {
> > safe_file_scanf(file, lineno, cleanup_fn, proc_path,
> > - "%*i %*s %c", &cur_state);
> > + "%*[^)]%*c %c", &cur_state);
>
> Obviously correct, thanks.
>
> Reviewed-by: Petr Vorel <pvorel@suse.cz>
>
> But there is also the same issue in lib/tst_thread_state.c,
> I guess it applies to that. Li, could you please also fix it before merge?
>
Good point, applied with the tst_thread_state change.
>
> Also, Andrea, you added tst_thread_state_wait() and
> TST_THREAD_STATE_WAIT() for
> futex_waitv0[23] related tests [1], but it's now not used anywhere due
> Jan's
> changes [2] [3]. I wonder if it's still useful or whether we should remove
> it.
>
Agreed with Jan, it is still useful in the future.
--
Regards,
Li Wang
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2024-02-07 2:21 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-02-05 10:26 [LTP] [PATCH] process_state: Enhancement of process state detection Li Wang
2024-02-06 11:51 ` Petr Vorel
2024-02-06 12:59 ` Jan Stancek
2024-02-07 2:21 ` Li Wang
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox