On Mon, 23 Dec 2013 14:22:24 +0100, Mark Wielaard wrote:
[...]
> --- a/libdwfl/linux-pid-attach.c
> +++ b/libdwfl/linux-pid-attach.c
> @@ -301,6 +301,27 @@ bool
> internal_function
> __libdwfl_attach_state_for_pid (Dwfl *dwfl, pid_t pid)
> {
> + char buffer[36];
> + FILE *procfile;
> +
> + /* Make sure to report the actual PID (thread group leader) to
> + dwfl_attach_state. */
> + snprintf (buffer, sizeof (buffer), "/proc/%ld/status", (long) pid);
> + procfile = fopen (buffer, "r");
> + if (procfile == NULL)
> + return false;
> +
> + while (fgets (buffer, sizeof (buffer), procfile) != NULL)
> + if (strncmp (buffer, "Tgid:", 5) == 0)
It works in practice but I do not find the code too much safe.
'buffer' is too small, /proc/*/status have lines longer than 36 chars,
therefore strncmp will be applied in middle of lines. Fortunately Tgid: is
present before the longer lines (but will it always be so?). The first
'Name:' line is max. 22 bytes incl. '\0'.
> + {
> + pid = atoi (&buffer[5]);
> + break;
> + }
> + fclose (procfile);
> +
> + if (pid == 0)
> + return false;
I do not understand this conditional. If "Tgid:" was not found PID will be
the user-specified TID, not 0.
> +
> char dirname[64];
> int i = snprintf (dirname, sizeof (dirname), "/proc/%ld/task", (long) pid);
> assert (i > 0 && i < (ssize_t) sizeof (dirname) - 1);
[...]
Otherwise OK with me.
Thanks,
Jan
