Re: [PATCH v2 5/7] ls-files: fix a trivial dir_clear() leak

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx> writes:

> -       if (ps_matched) {
> -               int bad;
> -               bad = report_path_error(ps_matched, &pathspec);
> -               if (bad)
> -                       fprintf(stderr, "Did you forget to 'git add'?\n");
> -
> -               return bad ? 1 : 0;
> +       if (ps_matched && report_path_error(ps_matched, &pathspec)) {
> +               fprintf(stderr, "Did you forget to 'git add'?\n");
> +               ret = 1;
>         }
>  
>         dir_clear(&dir);
>         free(max_prefix);
> -       return 0;
> +       return ret;
>  }
>
> Doesn't make much sense, but I can re-roll with it if you feel strongly
> about it. I think the current version is ready to be picked up.

I do not see where that "doesn't make much sense" comes from.  If it
does not make sense, I wouldn't have mentioned it.

> Yeah we should avoid refactoring-while-at-it, but in cases where a patch
> removes the only reason a nested if/if statement exists, unrolling it

And I do not quite see what "the only reason" is in this case, or
what it has to do with the restructuring, either.  Care to either
clarify, or fix the patch, or perhaps both?

Thanks.






[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux