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

Hey Evan,

> On Mon, Oct 3, 2016 at 3:10 PM, FRIGN <d...@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 <d...@frign.de>

Reply via email to