On 05/16/2011 04:23 PM, Mathieu Desnoyers wrote:
* Nils Carlson ([email protected]) wrote:
Previously libustctl would list all pids. This patch changes this
so only online pids are listed. This is done by appending on each
socket name the mtime from the proc/<pid> directory. This way a
socket can be checked to see if the appended mtime matches the
mtime of the proc dir for the current pid, thus allowing us to
distinguish between old and new socket files.
Hi Nils,
I'm OK with the approach. I have a few cosmetic comments below,
Signed-off-by: Nils Carlson<[email protected]>
---
libust/tracectl.c | 9 +++-
libustcomm/ustcomm.c | 150 ++++++++++++++++++++++++++++++++++++++++++++-----
libustcomm/ustcomm.h | 7 ++
libustctl/libustctl.c | 18 +-----
4 files changed, 153 insertions(+), 31 deletions(-)
diff --git a/libust/tracectl.c b/libust/tracectl.c
index bc0a07c..fb9130c 100644
--- a/libust/tracectl.c
+++ b/libust/tracectl.c
@@ -1228,12 +1228,19 @@ static struct ustcomm_sock * init_app_socket(int
epoll_fd)
char *dir_name, *sock_name;
int result;
struct ustcomm_sock *sock = NULL;
+ time_t mtime;
dir_name = ustcomm_user_sock_dir();
if (!dir_name)
return NULL;
- result = asprintf(&sock_name, "%s/%d", dir_name, (int)getpid());
+ mtime = ustcomm_pid_st_mtime(getpid());
+ if (!mtime) {
+ goto free_dir_name;
+ }
+
+ result = asprintf(&sock_name, "%s/%d.%ld", dir_name,
+ (int)getpid(), (long)mtime);
if (result< 0) {
ERR("string overflow allocating socket name, "
"UST thread bailing");
diff --git a/libustcomm/ustcomm.c b/libustcomm/ustcomm.c
index ed6d8f1..17bd9d9 100644
--- a/libustcomm/ustcomm.c
+++ b/libustcomm/ustcomm.c
@@ -22,6 +22,7 @@
#include<sys/types.h>
#include<signal.h>
#include<errno.h>
+#include<limits.h>
#include<sys/socket.h>
#include<sys/un.h>
#include<unistd.h>
@@ -550,6 +551,125 @@ char *ustcomm_user_sock_dir(void)
return sock_dir;
}
+static int time_and_pid_from_socket_name(char *sock_name, unsigned long *time,
+ pid_t *pid)
+{
+ char *saveptr, *pid_m_time_str;
+ char *sock_basename = strdup(basename(sock_name));
+
+ if (!sock_basename) {
+ return -1;
+ }
+
+ /* This is the pid */
+ pid_m_time_str = strtok_r(sock_basename, ".",&saveptr);
+ if (!pid_m_time_str) {
+ goto out_err;
+ }
+
+ errno = 0;
+ *pid = (pid_t)strtoul(pid_m_time_str, NULL, 10);
+ if (errno) {
Errno is a TLS variable, while the value returned by strtoul is directly
accessible from registers. So I would tend to think that using errno all
the time will bloat our code, causing TLS accesses when not necessary.
And I fear that on systems lacking TLS support (such as Android), they
might simply use a shared variable modified concurrently across threads,
which could be racy. So I would prefer to use the return value of the
functions to trigger the error, and use the errno value only to print
the error message in more details. Otherwise, I'm worried that we might
end up either missing errors or triggering on a false-positive from the
wrong thread.
Please read the man page for strtoul. Functions such as this one don't
return anything to indicate errors, errno is the only way to check.
+ goto out_err;
+ }
+
+ /* This should be the time-stamp */
+ pid_m_time_str = strtok_r(NULL, ".",&saveptr);
+ if (!pid_m_time_str) {
+ goto out_err;
+ }
+
+ errno = 0;
+ *time = strtoul(pid_m_time_str, NULL, 10);
+ if (errno) {
+ goto out_err;
+ }
+
+ return 0;
+
+out_err:
+ free(sock_basename);
+ return -1;
+}
+
+time_t ustcomm_pid_st_mtime(pid_t pid)
+{
+ struct stat proc_stat;
+ char proc_name[PATH_MAX + 1];
+
+ if (snprintf(proc_name, PATH_MAX, "/proc/%ld", (long)pid)< 0) {
+ return 0;
+ }
+
+ if (stat(proc_name,&proc_stat)) {
+ return 0;
+ }
+
+ return proc_stat.st_mtime;
+}
+
+int ustcomm_is_socket_live(char *sock_name, pid_t *read_pid)
+{
+ time_t time_from_pid;
+ unsigned long time_from_sock;
+ pid_t pid;
+
+ if (time_and_pid_from_socket_name(sock_name,&time_from_sock,&pid)) {
+ return 0;
+ }
+
+ if (read_pid) {
+ *read_pid = pid;
+ }
+
+ time_from_pid = ustcomm_pid_st_mtime(pid);
+ if (!time_from_pid) {
+ return 0;
+ }
+
+ if (((unsigned long)time_from_pid) == time_from_sock) {
Please use: (unsigned long) time_from_pid (with the space) (typical in
kernel coding style, which we try to follow).
Done, though as usual this isn't consistent with already existing code.
+ return 1;
+ }
+
+ return 0;
+}
+
+static int ustcomm_get_sock_name(char *dir_name, pid_t pid, char *sock_name)
+{
+ struct dirent *dirent;
+ char sock_path_base[101];
+ int len;
+ DIR *dir = opendir(dir_name);
+
+ snprintf(sock_path_base, 100, "%ld.", (long)pid);
+ len = strlen(sock_path_base);
+
+ while ((dirent = readdir(dir))) {
+ if (!strcmp(dirent->d_name, ".") ||
+ !strcmp(dirent->d_name, "..") ||
+ !strcmp(dirent->d_name, "ust-consumer") ||
+ dirent->d_type == DT_DIR ||
+ strncmp(dirent->d_name, sock_path_base, len)) {
+ continue;
+ }
+
+ if (ustcomm_is_socket_live(dirent->d_name, NULL)) {
+ printf("found %s\n", dirent->d_name);
Should this message be only printed in verbose mode ?
Darn, debug! :-) Will fix the rest.
/Nils
_______________________________________________
ltt-dev mailing list
[email protected]
http://lists.casi.polymtl.ca/cgi-bin/mailman/listinfo/ltt-dev
