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

Ævar Arnfjörð Bjarmason [off-list ref] writes:

On Wed, Oct 13 2021, Junio C Hamano wrote:

quoted

Ævar Arnfjörð Bjarmason [off-list ref] writes:

quoted

-       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;
        }

 ...

I mean that we generally don't write code like:

	if (x) {
		if (y) {
			fprintf(...);
			ret = 1;
		}
	}

And instead write:

	if (x && y) {
		fprintf(...);
		ret = 1;
	}

So aside from the specific change here I thought your objection would
also apply to e.g. removing braces from a standalone "if" as it's
reduced to a one statement body, which we we generally do.

Yes, but in a "plug-leak-only" fix, I would have expected

	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;
+		ret = bad ? 1 : 0;

	}

Doing anything more than that is "while at it clean-up".