On Sat, 26 Mar 2016 12:34:52 +0100
Hiltjo Posthuma <[email protected]> wrote:

> On Sat, Mar 26, 2016 at 12:30:50AM +0100, Mattias Andr??e
> wrote:
> > Signed-off-by: Mattias Andr??e <[email protected]>
> > diff --git a/pwdx.c b/pwdx.c
> > new file mode 100644
> > index 0000000..b9836b7
> > --- /dev/null
> > +++ b/pwdx.c
> > @@ -0,0 +1,53 @@
> > +/* See LICENSE file for copyright and license details.
> > */ +#include <limits.h>
> > +#include <stdio.h>
> > +#include <string.h>
> > +#include <errno.h>
> > +#include <unistd.h>
> > +  
> 
> Sort includes.
> 
> > +#include "util.h"
> > +
> > +static void
> > +usage(void)
> > +{
> > +   eprintf("usage: %s pid...\n", argv0);
> > +}
> > +
> > +int
> > +main(int argc, char *argv[])
> > +{
> > +   int ret = 0;
> > +   char path[sizeof("/proc//cwd") + 3 *
> > sizeof(pid_t)];  
> 
> Just use PATH_MAX here.
> 
> > +   char target[PATH_MAX + sizeof(" (deleted)")];  
> 
> Why + sizeof(" (deleted)") ?

If the cwd of that pid is almost PATH_MAX long,
and have been deleted, it may overflow otherwise
since kernel append " (deleted)" to deleted paths.

> 
> > +
> > +   for (; argc > 0; argc--, argv++) {
> > +           if (strlen(*argv) > 3 * sizeof(pid_t))
> > {
> > +                   fprintf(stderr, "%s: No such
> > process\n", *argv);
> > +                   ret = 1;
> > +                   continue;
> > +           }
> > +           sprintf(path, "/proc/%s/cwd", *argv);  
> 
> Please use snprintf here and check for -1 and truncation. 

Normally I would disagree of using snprintf when there is
no way sprintf can write outside the buffer, but in this
case I agree since `strlen(*argv) > 3 * sizeof(pid_t)`
becomes unnecessary.

> 
> > +           n = readlink(path, target,
> > sizeof(target) - 1);
> > +           if (n >= 0) {
> > +                   target[n] = '\0';
> > +                   printf("%s: %s\n", *argv,
> > target);
> > +           } else if (errno == ENOENT) {
> > +                   fprintf(stderr, "%s: No such
> > process\n", *argv);
> > +                   ret = 1;  
> 
> Here you can use eprintf().
> 
> > +           } else {
> > +                   perror(*argv);
> > +                   ret = 1;
> > +           }
> > +   }
> > +
> > +   return ret;
> > +}
> > -- 
> > 2.7.3
> > 
> >   
> 
> Kind regards,
> Hiltjo
> 

Attachment: pgpXTdYZ1ImAY.pgp
Description: OpenPGP digital signature

Reply via email to