On Wed, Dec 25, 2013 at 08:23:25PM +0100, Jan Kratochvil wrote:
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'.
Yeah, changed to use getline, just in case.
We should avoid fgets in general, only getline really is safe.
> + {
> + 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.
It would be zero in the very unlikely case atoi doesn't see any digits.
Otherwise OK with me.
Thanks, pushed.
Minus the one line src/stack change, which I moved into the
stack: Simplify argument parsing commit on mjw/pending.
Mark