Re: [hackers] [sbase][patch]find: copy path before using basename

From: Laslo Hunhold <dev_AT_frign.de>
Date: Fri, 14 Oct 2016 13:15:52 +0200

On Wed, 5 Oct 2016 15:41:55 -0700
Evan Gates <evan.gates_AT_gmail.com> wrote:

Hey Evan,

> On Mon, Oct 3, 2016 at 3:10 PM, FRIGN <dev_AT_frign.de> wrote:
> > Please don't use VLA's. Use estrdup() in this case.
>
> sbase-find_basename2.diff: revised patch without VLAs
> sbase-find_noVLAs.diff: path to remove all VLAs in find

let me give you some feedback on the patches.

### sbase-find_basename2.diff

 static int
 pri_name(struct arg *arg)
 {
- return !fnmatch((char *)arg->extra.p, basename(arg->path), 0);
+ char *path = estrdup(arg->path);
+ int ret = !fnmatch((char *)arg->extra.p, basename(path), 0);
+ free(path);
+ return ret;
 }

I'd strictly separate declaration from initialization so it is easier
to see why we even need "ret" here.

static int
pri_name(struct arg *arg)
{
        int ret;
        char *path;

        path = estrdup(arg->path);
        ret = !fnmatch((char *)arg->extra.p, basename(path), 0);
        free(path);
        
        return ret;
}

### sbase-find_noVLAs.diff

This already looks good, but I really am a big fan of initializing
variables as late as possible.

+ struct tok *tok, *rpn, *out, **top,
+ *infix = emalloc((2 * argc + 1) * sizeof(*infix)),
+ **stack = emalloc(argc * sizeof(*stack));

Use ereallocarray() for cases like these, it also makes the code more
readable.
Thanks for your contributions!

Cheers

Laslo

-- 
Laslo Hunhold <dev_AT_frign.de>
Received on Fri Oct 14 2016 - 13:15:52 CEST

This archive was generated by hypermail 2.3.0 : Fri Oct 14 2016 - 13:24:23 CEST