* [ltt-dev] [UST PATCH] libustctl: cleanup ustctl_get_online_pids functions
2011-04-04 9:47 [ltt-dev] [UST PATCH] libustctl: cleanup ustctl_get_online_pids functions Nils Carlson
@ 2011-04-04 9:24 ` Nils Carlson
0 siblings, 0 replies; 4+ messages in thread
From: Nils Carlson @ 2011-04-04 9:24 UTC (permalink / raw)
merged.
/Nils
On 04/04/2011 11:47 AM, Nils Carlson wrote:
> Cleanup the ustctl_get_online_pids functions, checking more errors
> and making the allocation strategy sane.
>
> Signed-off-by: Nils Carlson<nils.carlson at ericsson.com>
> ---
> libustctl/libustctl.c | 91 +++++++++++++++++++++++++++++++++++++------------
> 1 files changed, 69 insertions(+), 22 deletions(-)
>
> diff --git a/libustctl/libustctl.c b/libustctl/libustctl.c
> index 356d5ef..c781283 100644
> --- a/libustctl/libustctl.c
> +++ b/libustctl/libustctl.c
> @@ -88,11 +88,29 @@ int ustctl_connect_pid(pid_t pid)
> return sock;
> }
>
> -static void get_pids_in_dir(DIR *dir, pid_t **pid_list,
> +static int realloc_pid_list(pid_t **pid_list, unsigned int *pid_list_size)
> +{
> + pid_t *new_pid_list;
> + unsigned int new_pid_list_size = 2 * *pid_list_size;
> +
> + new_pid_list = realloc(*pid_list,
> + new_pid_list_size * sizeof(pid_t));
> + if (!*new_pid_list) {
> + return -1;
> + }
> +
> + *pid_list = new_pid_list;
> + *pid_list_size = new_pid_list_size;
> +
> + return 0;
> +}
> +
> +static int get_pids_in_dir(DIR *dir, pid_t **pid_list,
> + unsigned int *pid_list_index,
> unsigned int *pid_list_size)
> {
> struct dirent *dirent;
> - unsigned int read_pid;
> + long read_pid;
>
> while ((dirent = readdir(dir))) {
> if (!strcmp(dirent->d_name, ".") ||
> @@ -103,29 +121,40 @@ static void get_pids_in_dir(DIR *dir, pid_t **pid_list,
> continue;
> }
>
> - sscanf(dirent->d_name, "%u",&read_pid);
> + errno = 0;
> + read_pid = strtol(dirent->d_name, NULL, 10);
> + if (errno) {
> + continue;
> + }
>
> - (*pid_list)[*pid_list_size - 1] = read_pid;
> - /* FIXME: Here we previously called pid_is_online, which
> + /*
> + * FIXME: Here we previously called pid_is_online, which
> * always returned 1, now I replaced it with just 1.
> * We need to figure out an intelligent way of solving
> * this, maybe connect-disconnect.
> */
> if (1) {
> - (*pid_list_size)++;
> - *pid_list = realloc(*pid_list,
> - *pid_list_size * sizeof(pid_t));
> +
> + (*pid_list)[(*pid_list_index)++] = read_pid;
> +
> + if (*pid_list_index == *pid_list_size) {
> + if (realloc_pid_list(pid_list, pid_list_size)) {
> + return -1;
> + }
> + }
> }
> }
>
> - (*pid_list)[*pid_list_size - 1] = 0; /* Array end */
> + (*pid_list)[*pid_list_index] = 0; /* Array end */
> +
> + return 0;
> }
>
> static pid_t *get_pids_non_root(void)
> {
> char *dir_name;
> DIR *dir;
> - unsigned int pid_list_size = 1;
> + unsigned int pid_list_index = 0, pid_list_size = 1;
> pid_t *pid_list = NULL;
>
> dir_name = ustcomm_user_sock_dir();
> @@ -143,13 +172,9 @@ static pid_t *get_pids_non_root(void)
> goto close_dir;
> }
>
> - get_pids_in_dir(dir,&pid_list,&pid_list_size);
> -
> - if (pid_list[0] == 0) {
> - /* No PID at all */
> - free(pid_list);
> - pid_list = NULL;
> - goto close_dir;
> + if (get_pids_in_dir(dir,&pid_list,&pid_list_index,&pid_list_size)) {
> + /* if any errors are encountered, force freeing of the list */
> + pid_list[0] = 0;
> }
>
> close_dir:
> @@ -165,9 +190,10 @@ static pid_t *get_pids_root(void)
> {
> char *dir_name;
> DIR *tmp_dir, *dir;
> - unsigned int pid_list_size = 1;
> + unsigned int pid_list_index = 0, pid_list_size = 1;
> pid_t *pid_list = NULL;
> struct dirent *dirent;
> + int result;
>
> tmp_dir = opendir(USER_TMP_DIR);
> if (!tmp_dir) {
> @@ -184,7 +210,8 @@ static pid_t *get_pids_root(void)
> if (!strncmp(dirent->d_name, USER_SOCK_DIR_BASE,
> strlen(USER_SOCK_DIR_BASE))) {
>
> - if (asprintf(&dir_name, USER_TMP_DIR "/%s", dirent->d_name)< 0) {
> + if (asprintf(&dir_name, USER_TMP_DIR "/%s",
> + dirent->d_name)< 0) {
> goto close_tmp_dir;
> }
>
> @@ -196,9 +223,19 @@ static pid_t *get_pids_root(void)
> continue;
> }
>
> - get_pids_in_dir(dir,&pid_list,&pid_list_size);
> + result = get_pids_in_dir(dir,&pid_list,&pid_list_index,
> + &pid_list_size);
>
> closedir(dir);
> +
> + if (result) {
> + /*
> + * if any errors are encountered,
> + * force freeing of the list
> + */
> + pid_list[0] = 0;
> + break;
> + }
> }
> }
>
> @@ -210,11 +247,21 @@ close_tmp_dir:
>
> pid_t *ustctl_get_online_pids(void)
> {
> + pid_t *pid_list;
> +
> if (geteuid()) {
> - return get_pids_non_root();
> + pid_list = get_pids_non_root();
> } else {
> - return get_pids_root();
> + pid_list = get_pids_root();
> + }
> +
> + if (pid_list&& pid_list[0] == 0) {
> + /* No PID at all */
> + free(pid_list);
> + pid_list = NULL;
> }
> +
> + return pid_list;
> }
>
> /**
^ permalink raw reply [flat|nested] 4+ messages in thread
* [ltt-dev] [UST PATCH] libustctl: cleanup ustctl_get_online_pids functions
@ 2011-04-04 9:47 Nils Carlson
2011-04-04 9:24 ` Nils Carlson
0 siblings, 1 reply; 4+ messages in thread
From: Nils Carlson @ 2011-04-04 9:47 UTC (permalink / raw)
Cleanup the ustctl_get_online_pids functions, checking more errors
and making the allocation strategy sane.
Signed-off-by: Nils Carlson <nils.carlson at ericsson.com>
---
libustctl/libustctl.c | 91 +++++++++++++++++++++++++++++++++++++------------
1 files changed, 69 insertions(+), 22 deletions(-)
diff --git a/libustctl/libustctl.c b/libustctl/libustctl.c
index 356d5ef..c781283 100644
--- a/libustctl/libustctl.c
+++ b/libustctl/libustctl.c
@@ -88,11 +88,29 @@ int ustctl_connect_pid(pid_t pid)
return sock;
}
-static void get_pids_in_dir(DIR *dir, pid_t **pid_list,
+static int realloc_pid_list(pid_t **pid_list, unsigned int *pid_list_size)
+{
+ pid_t *new_pid_list;
+ unsigned int new_pid_list_size = 2 * *pid_list_size;
+
+ new_pid_list = realloc(*pid_list,
+ new_pid_list_size * sizeof(pid_t));
+ if (!*new_pid_list) {
+ return -1;
+ }
+
+ *pid_list = new_pid_list;
+ *pid_list_size = new_pid_list_size;
+
+ return 0;
+}
+
+static int get_pids_in_dir(DIR *dir, pid_t **pid_list,
+ unsigned int *pid_list_index,
unsigned int *pid_list_size)
{
struct dirent *dirent;
- unsigned int read_pid;
+ long read_pid;
while ((dirent = readdir(dir))) {
if (!strcmp(dirent->d_name, ".") ||
@@ -103,29 +121,40 @@ static void get_pids_in_dir(DIR *dir, pid_t **pid_list,
continue;
}
- sscanf(dirent->d_name, "%u", &read_pid);
+ errno = 0;
+ read_pid = strtol(dirent->d_name, NULL, 10);
+ if (errno) {
+ continue;
+ }
- (*pid_list)[*pid_list_size - 1] = read_pid;
- /* FIXME: Here we previously called pid_is_online, which
+ /*
+ * FIXME: Here we previously called pid_is_online, which
* always returned 1, now I replaced it with just 1.
* We need to figure out an intelligent way of solving
* this, maybe connect-disconnect.
*/
if (1) {
- (*pid_list_size)++;
- *pid_list = realloc(*pid_list,
- *pid_list_size * sizeof(pid_t));
+
+ (*pid_list)[(*pid_list_index)++] = read_pid;
+
+ if (*pid_list_index == *pid_list_size) {
+ if (realloc_pid_list(pid_list, pid_list_size)) {
+ return -1;
+ }
+ }
}
}
- (*pid_list)[*pid_list_size - 1] = 0; /* Array end */
+ (*pid_list)[*pid_list_index] = 0; /* Array end */
+
+ return 0;
}
static pid_t *get_pids_non_root(void)
{
char *dir_name;
DIR *dir;
- unsigned int pid_list_size = 1;
+ unsigned int pid_list_index = 0, pid_list_size = 1;
pid_t *pid_list = NULL;
dir_name = ustcomm_user_sock_dir();
@@ -143,13 +172,9 @@ static pid_t *get_pids_non_root(void)
goto close_dir;
}
- get_pids_in_dir(dir, &pid_list, &pid_list_size);
-
- if (pid_list[0] == 0) {
- /* No PID at all */
- free(pid_list);
- pid_list = NULL;
- goto close_dir;
+ if (get_pids_in_dir(dir, &pid_list, &pid_list_index, &pid_list_size)) {
+ /* if any errors are encountered, force freeing of the list */
+ pid_list[0] = 0;
}
close_dir:
@@ -165,9 +190,10 @@ static pid_t *get_pids_root(void)
{
char *dir_name;
DIR *tmp_dir, *dir;
- unsigned int pid_list_size = 1;
+ unsigned int pid_list_index = 0, pid_list_size = 1;
pid_t *pid_list = NULL;
struct dirent *dirent;
+ int result;
tmp_dir = opendir(USER_TMP_DIR);
if (!tmp_dir) {
@@ -184,7 +210,8 @@ static pid_t *get_pids_root(void)
if (!strncmp(dirent->d_name, USER_SOCK_DIR_BASE,
strlen(USER_SOCK_DIR_BASE))) {
- if (asprintf(&dir_name, USER_TMP_DIR "/%s", dirent->d_name) < 0) {
+ if (asprintf(&dir_name, USER_TMP_DIR "/%s",
+ dirent->d_name) < 0) {
goto close_tmp_dir;
}
@@ -196,9 +223,19 @@ static pid_t *get_pids_root(void)
continue;
}
- get_pids_in_dir(dir, &pid_list, &pid_list_size);
+ result = get_pids_in_dir(dir, &pid_list, &pid_list_index,
+ &pid_list_size);
closedir(dir);
+
+ if (result) {
+ /*
+ * if any errors are encountered,
+ * force freeing of the list
+ */
+ pid_list[0] = 0;
+ break;
+ }
}
}
@@ -210,11 +247,21 @@ close_tmp_dir:
pid_t *ustctl_get_online_pids(void)
{
+ pid_t *pid_list;
+
if (geteuid()) {
- return get_pids_non_root();
+ pid_list = get_pids_non_root();
} else {
- return get_pids_root();
+ pid_list = get_pids_root();
+ }
+
+ if (pid_list && pid_list[0] == 0) {
+ /* No PID at all */
+ free(pid_list);
+ pid_list = NULL;
}
+
+ return pid_list;
}
/**
--
1.7.1
^ permalink raw reply [flat|nested] 4+ messages in thread
* [ltt-dev] [UST PATCH] libustctl: cleanup ustctl_get_online_pids functions
2011-04-01 12:45 Nils Carlson
@ 2011-04-01 15:01 ` Mathieu Desnoyers
0 siblings, 0 replies; 4+ messages in thread
From: Mathieu Desnoyers @ 2011-04-01 15:01 UTC (permalink / raw)
* Nils Carlson (nils.carlson at ericsson.com) wrote:
> Cleanup the ustctl_get_online_pids functions, checking more errors
> and making the allocation strategy sane.
>
> Signed-off-by: Nils Carlson <nils.carlson at ericsson.com>
> ---
> libustctl/libustctl.c | 65 +++++++++++++++++++++++++++++++++++++-----------
> 1 files changed, 50 insertions(+), 15 deletions(-)
>
> diff --git a/libustctl/libustctl.c b/libustctl/libustctl.c
> index 356d5ef..1911434 100644
> --- a/libustctl/libustctl.c
> +++ b/libustctl/libustctl.c
> @@ -88,11 +88,12 @@ int ustctl_connect_pid(pid_t pid)
> return sock;
> }
>
> -static void get_pids_in_dir(DIR *dir, pid_t **pid_list,
> +static int get_pids_in_dir(DIR *dir, pid_t **pid_list,
> + unsigned int *pid_list_index,
> unsigned int *pid_list_size)
> {
> struct dirent *dirent;
> - unsigned int read_pid;
> + long read_pid;
>
> while ((dirent = readdir(dir))) {
> if (!strcmp(dirent->d_name, ".") ||
> @@ -103,29 +104,43 @@ static void get_pids_in_dir(DIR *dir, pid_t **pid_list,
> continue;
> }
>
> - sscanf(dirent->d_name, "%u", &read_pid);
> + errno = 0;
> + read_pid = strtol(dirent->d_name, NULL, 10);
> + if (errno) {
> + continue;
> + }
>
> - (*pid_list)[*pid_list_size - 1] = read_pid;
> - /* FIXME: Here we previously called pid_is_online, which
> + /*
> + * FIXME: Here we previously called pid_is_online, which
> * always returned 1, now I replaced it with just 1.
> * We need to figure out an intelligent way of solving
> * this, maybe connect-disconnect.
> */
> if (1) {
> - (*pid_list_size)++;
> - *pid_list = realloc(*pid_list,
> - *pid_list_size * sizeof(pid_t));
> +
> + (*pid_list)[(*pid_list_index)++] = read_pid;
> +
> + if (*pid_list_index == *pid_list_size) {
> + *pid_list_size *= 2;
> + *pid_list = realloc(*pid_list,
> + *pid_list_size * sizeof(pid_t));
> + if (!*pid_list) {
> + return -1;
Instead of returning here and duplicating the error handing code in the
caller, can you instead do:
goto error;
> + }
> + }
> }
> }
>
> - (*pid_list)[*pid_list_size - 1] = 0; /* Array end */
> + (*pid_list)[*pid_list_index] = 0; /* Array end */
> +
> + return 0;
error:
(*pid_list)[0] = 0;
return -1;
Thanks,
Mathieu
> }
>
> static pid_t *get_pids_non_root(void)
> {
> char *dir_name;
> DIR *dir;
> - unsigned int pid_list_size = 1;
> + unsigned int pid_list_index = 0, pid_list_size = 1;
> pid_t *pid_list = NULL;
>
> dir_name = ustcomm_user_sock_dir();
> @@ -143,13 +158,15 @@ static pid_t *get_pids_non_root(void)
> goto close_dir;
> }
>
> - get_pids_in_dir(dir, &pid_list, &pid_list_size);
> + if (get_pids_in_dir(dir, &pid_list, &pid_list_index, &pid_list_size)) {
> + /* if any errors are encountered, force freeing of the list */
> + pid_list[0] = 0;
> + }
>
> if (pid_list[0] == 0) {
> /* No PID at all */
> free(pid_list);
> pid_list = NULL;
> - goto close_dir;
> }
>
> close_dir:
> @@ -165,9 +182,10 @@ static pid_t *get_pids_root(void)
> {
> char *dir_name;
> DIR *tmp_dir, *dir;
> - unsigned int pid_list_size = 1;
> + unsigned int pid_list_index = 0, pid_list_size = 1;
> pid_t *pid_list = NULL;
> struct dirent *dirent;
> + int result;
>
> tmp_dir = opendir(USER_TMP_DIR);
> if (!tmp_dir) {
> @@ -184,7 +202,8 @@ static pid_t *get_pids_root(void)
> if (!strncmp(dirent->d_name, USER_SOCK_DIR_BASE,
> strlen(USER_SOCK_DIR_BASE))) {
>
> - if (asprintf(&dir_name, USER_TMP_DIR "/%s", dirent->d_name) < 0) {
> + if (asprintf(&dir_name, USER_TMP_DIR "/%s",
> + dirent->d_name) < 0) {
> goto close_tmp_dir;
> }
>
> @@ -196,12 +215,28 @@ static pid_t *get_pids_root(void)
> continue;
> }
>
> - get_pids_in_dir(dir, &pid_list, &pid_list_size);
> + result = get_pids_in_dir(dir, &pid_list, &pid_list_index,
> + &pid_list_size);
>
> closedir(dir);
> +
> + if (result) {
> + /*
> + * if any errors are encountered,
> + * force freeing of the list
> + */
> + pid_list[0] = 0;
> + break;
> + }
> }
> }
>
> + if (pid_list[0] == 0) {
> + /* No PID at all */
> + free(pid_list);
> + pid_list = NULL;
> + }
> +
> close_tmp_dir:
> closedir(tmp_dir);
>
> --
> 1.7.1
>
>
> _______________________________________________
> ltt-dev mailing list
> ltt-dev at lists.casi.polymtl.ca
> http://lists.casi.polymtl.ca/cgi-bin/mailman/listinfo/ltt-dev
>
--
Mathieu Desnoyers
Operating System Efficiency R&D Consultant
EfficiOS Inc.
http://www.efficios.com
^ permalink raw reply [flat|nested] 4+ messages in thread
* [ltt-dev] [UST PATCH] libustctl: cleanup ustctl_get_online_pids functions
@ 2011-04-01 12:45 Nils Carlson
2011-04-01 15:01 ` Mathieu Desnoyers
0 siblings, 1 reply; 4+ messages in thread
From: Nils Carlson @ 2011-04-01 12:45 UTC (permalink / raw)
Cleanup the ustctl_get_online_pids functions, checking more errors
and making the allocation strategy sane.
Signed-off-by: Nils Carlson <nils.carlson at ericsson.com>
---
libustctl/libustctl.c | 65 +++++++++++++++++++++++++++++++++++++-----------
1 files changed, 50 insertions(+), 15 deletions(-)
diff --git a/libustctl/libustctl.c b/libustctl/libustctl.c
index 356d5ef..1911434 100644
--- a/libustctl/libustctl.c
+++ b/libustctl/libustctl.c
@@ -88,11 +88,12 @@ int ustctl_connect_pid(pid_t pid)
return sock;
}
-static void get_pids_in_dir(DIR *dir, pid_t **pid_list,
+static int get_pids_in_dir(DIR *dir, pid_t **pid_list,
+ unsigned int *pid_list_index,
unsigned int *pid_list_size)
{
struct dirent *dirent;
- unsigned int read_pid;
+ long read_pid;
while ((dirent = readdir(dir))) {
if (!strcmp(dirent->d_name, ".") ||
@@ -103,29 +104,43 @@ static void get_pids_in_dir(DIR *dir, pid_t **pid_list,
continue;
}
- sscanf(dirent->d_name, "%u", &read_pid);
+ errno = 0;
+ read_pid = strtol(dirent->d_name, NULL, 10);
+ if (errno) {
+ continue;
+ }
- (*pid_list)[*pid_list_size - 1] = read_pid;
- /* FIXME: Here we previously called pid_is_online, which
+ /*
+ * FIXME: Here we previously called pid_is_online, which
* always returned 1, now I replaced it with just 1.
* We need to figure out an intelligent way of solving
* this, maybe connect-disconnect.
*/
if (1) {
- (*pid_list_size)++;
- *pid_list = realloc(*pid_list,
- *pid_list_size * sizeof(pid_t));
+
+ (*pid_list)[(*pid_list_index)++] = read_pid;
+
+ if (*pid_list_index == *pid_list_size) {
+ *pid_list_size *= 2;
+ *pid_list = realloc(*pid_list,
+ *pid_list_size * sizeof(pid_t));
+ if (!*pid_list) {
+ return -1;
+ }
+ }
}
}
- (*pid_list)[*pid_list_size - 1] = 0; /* Array end */
+ (*pid_list)[*pid_list_index] = 0; /* Array end */
+
+ return 0;
}
static pid_t *get_pids_non_root(void)
{
char *dir_name;
DIR *dir;
- unsigned int pid_list_size = 1;
+ unsigned int pid_list_index = 0, pid_list_size = 1;
pid_t *pid_list = NULL;
dir_name = ustcomm_user_sock_dir();
@@ -143,13 +158,15 @@ static pid_t *get_pids_non_root(void)
goto close_dir;
}
- get_pids_in_dir(dir, &pid_list, &pid_list_size);
+ if (get_pids_in_dir(dir, &pid_list, &pid_list_index, &pid_list_size)) {
+ /* if any errors are encountered, force freeing of the list */
+ pid_list[0] = 0;
+ }
if (pid_list[0] == 0) {
/* No PID at all */
free(pid_list);
pid_list = NULL;
- goto close_dir;
}
close_dir:
@@ -165,9 +182,10 @@ static pid_t *get_pids_root(void)
{
char *dir_name;
DIR *tmp_dir, *dir;
- unsigned int pid_list_size = 1;
+ unsigned int pid_list_index = 0, pid_list_size = 1;
pid_t *pid_list = NULL;
struct dirent *dirent;
+ int result;
tmp_dir = opendir(USER_TMP_DIR);
if (!tmp_dir) {
@@ -184,7 +202,8 @@ static pid_t *get_pids_root(void)
if (!strncmp(dirent->d_name, USER_SOCK_DIR_BASE,
strlen(USER_SOCK_DIR_BASE))) {
- if (asprintf(&dir_name, USER_TMP_DIR "/%s", dirent->d_name) < 0) {
+ if (asprintf(&dir_name, USER_TMP_DIR "/%s",
+ dirent->d_name) < 0) {
goto close_tmp_dir;
}
@@ -196,12 +215,28 @@ static pid_t *get_pids_root(void)
continue;
}
- get_pids_in_dir(dir, &pid_list, &pid_list_size);
+ result = get_pids_in_dir(dir, &pid_list, &pid_list_index,
+ &pid_list_size);
closedir(dir);
+
+ if (result) {
+ /*
+ * if any errors are encountered,
+ * force freeing of the list
+ */
+ pid_list[0] = 0;
+ break;
+ }
}
}
+ if (pid_list[0] == 0) {
+ /* No PID at all */
+ free(pid_list);
+ pid_list = NULL;
+ }
+
close_tmp_dir:
closedir(tmp_dir);
--
1.7.1
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2011-04-04 9:47 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-04-04 9:47 [ltt-dev] [UST PATCH] libustctl: cleanup ustctl_get_online_pids functions Nils Carlson
2011-04-04 9:24 ` Nils Carlson
-- strict thread matches above, loose matches on Subject: below --
2011-04-01 12:45 Nils Carlson
2011-04-01 15:01 ` Mathieu Desnoyers
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox