From: Florian Weimer <fw@deneb.enyo.de>
To: Adhemerval Zanella <adhemerval.zanella@linaro.org>
Cc: Florian Weimer <fweimer@redhat.com>, libc-alpha@sourceware.org
Subject: Re: [PATCH 1/3] support: Add support_process_state_wait
Date: Fri, 20 Dec 2019 21:32:14 +0100 [thread overview]
Message-ID: <87r20yn3g1.fsf@mid.deneb.enyo.de> (raw)
In-Reply-To: <7bb7f49e-084f-cd65-ad02-48660ffeae5c@linaro.org> (Adhemerval Zanella's message of "Fri, 20 Dec 2019 17:19:48 -0300")
* Adhemerval Zanella:
> diff --git a/support/process_state.h b/support/process_state.h
> new file mode 100644
> index 0000000000..d983197247
> --- /dev/null
> +++ b/support/process_state.h
> @@ -0,0 +1,45 @@
> +#ifndef SUPPORT_PROCESS_STATE_H
> +#define SUPPORT_PROCESS_STATE_H
> +
> +#include <sys/types.h>
I think you also need to include <time.h> for struct timespec,
strictly speaking.
> diff --git a/support/support_process_state.c b/support/support_process_state.c
> new file mode 100644
> index 0000000000..16c2c3642e
> --- /dev/null
> +++ b/support/support_process_state.c
> +/* Returns the character representing the task state from FD. */
> +static char find_state (int fd)
> +{
> + char statusbuf[strlen ("Name:\t")
> + + 64 /* task name. */
> + + strlen ("Umask:\t") /* optional. */
> + + 4 /* umask size. */
> + + strlen ("State:\t")
> + + 1 /* <state>, 1 char. */
> + + 1]; /* Final \0. */
> +
> + ssize_t ret = read (fd, statusbuf, sizeof (statusbuf) - 1);
> + TEST_VERIFY (ret >= 0);
> + statusbuf[ret] = '\0';
> +
> + char *statestr = strstr (statusbuf, "State:\t");
> + TEST_VERIFY (statestr != NULL);
> + char state;
> + int sret = sscanf (statestr, "%*s %c", &state);
> + TEST_VERIFY (sret == 1);
> +
> + return state;
> +}
I think the expectation here is that more things will be inserted
after Name:. As you note, Umask: is already a late arrival.
So if simplicity is a concern, I would just read 4 KiB or so, and then
use strstr with "\nState:\t" (note the \n). The sscanf seems
superfluous in this context. Every else heads in the direction of
getline …
I think it also makes sense to return 0 in case the state cannot be
determined, rather than an indeterminate value.
> +void
> +support_process_state_wait (pid_t pid, enum support_process_state state,
> + struct timespec *poll_ts)
> +{
> +#ifdef __linux__
> + /* For Linux it does a polling check on /proc/<pid>/status checking on
> + third field. */
> +
> + /* It mimics the kernel states from fs/proc/array.c */
> + static const struct process_states_t
> + {
> + enum support_process_state s;
> + char v;
> + } process_states[] = {
> + { support_process_state_running, 'R' },
> + { support_process_state_sleeping, 'S' },
> + { support_process_state_disk_sleep, 'D' },
> + { support_process_state_stopped, 'T' },
> + { support_process_state_tracing_stop, 't' },
> + { support_process_state_dead, 'X' },
> + { support_process_state_zombie, 'Z' },
> + { support_process_state_parked, 'P' },
> + };
> +
> + char proc_path[sizeof ("/proc/" + 3) * sizeof (pid_t) + sizeof ("/stat")
> + + 1];
> + snprintf (proc_path, sizeof (proc_path), "/proc/%i/status", pid);
> +
> + int fd = xopen (proc_path, O_RDONLY, 0600);
> +
> + for (;;)
> + {
> + char cur_state = find_state (fd);
> +
> + for (size_t i = 0; i < array_length (process_states); ++i)
> + if (state & process_states[i].s && cur_state == process_states[i].v)
> + goto found;
I missed that before, I think we should error out if the state is
completely unknown. Maybe use strchr on "RSDTtXZP" instead, and use
that for the shift? 8-)
next prev parent reply other threads:[~2019-12-20 20:33 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-11-20 14:15 [PATCH 1/3] support: Add support_process_state_wait Adhemerval Zanella
2019-11-20 14:15 ` [PATCH 2/3] posix: Refactor tst-waitid Adhemerval Zanella
2019-11-20 17:33 ` Joseph Myers
2019-11-20 19:42 ` Adhemerval Zanella
2019-12-20 12:36 ` Adhemerval Zanella
2019-11-20 14:15 ` [PATCH 3/3] posix: Remove posix waitid Adhemerval Zanella
2019-11-20 15:14 ` Christian Brauner
2019-11-20 17:14 ` Adhemerval Zanella
2019-12-20 12:36 ` Adhemerval Zanella
2019-12-20 12:35 ` [PATCH 1/3] support: Add support_process_state_wait Adhemerval Zanella
2019-12-20 14:09 ` Florian Weimer
2019-12-20 20:19 ` Adhemerval Zanella
2019-12-20 20:32 ` Florian Weimer [this message]
2019-12-26 14:20 ` Adhemerval Zanella
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: https://www.gnu.org/software/libc/involved.html
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87r20yn3g1.fsf@mid.deneb.enyo.de \
--to=fw@deneb.enyo.de \
--cc=adhemerval.zanella@linaro.org \
--cc=fweimer@redhat.com \
--cc=libc-alpha@sourceware.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).