On Fri, Aug 09, 2019 at 11:25:52AM +0000, brian m. carlson wrote:
> > > + if (!flush_attributes && patch->new_name) {
> > > + char *dummy =
> > > strip_path_suffix(patch->new_name, GITATTRIBUTES_FILE);
> >
> > It's a shame that 'strip_path_suffix' doesn't take a 'char *out', and
> > accept NULL for that (which would save us the assignment and subsequent
> > 'free'). In either case, this is certainly the appropriate function.
>
> Yeah, I felt the same way. We could refactor this out into two separate
> functions, one which returns an ssize_t and one which actually
> allocates, but I'm not sure it makes a huge amount of sense with just
> one caller. The allocation is relatively small, and I've tried to make
> sure it's called exactly once per patch so as not to be wasteful and
> inefficient.
I think you could do this with:
size_t len;
if (strip_suffix(patch->new_name, GITATTRIBUTES_FILE, &len) &&
len > 0 && is_dir_sep(patch->new_name[len-1]))
flush_attributes = 1;
Not sure if that is better or worse. It avoids the extra memory
boilerplate, but the logic is a slightly more subtle.
-Peff
