Re: [PATCH] Silence Valgrind about SelectConfigFiles()

Aleksander Alekseev <aleksander(at)tigerdata(dot)com> writes:
>> You didn't say what the test condition was, but from the patch
>> I suppose it's a case where SelectConfigFiles is erroring out?

> Steps to reproduce:
> ...

Huh ... don't quite see where in that recipe we'd reach a
SelectConfigFiles error exit.

> It's not exciting at all but given the fact that this is the only
> "definitely lost" error I got it's worth fixing IMO ("broken windows"
> and all that). I should have also pointed out that SelectConfigFiles()
> sometimes calls free(configdir) and sometimes doesn't, which is
> inconsistent.

Yeah, I noticed that too, and it does offend my inner neatnik.

Instead of what you did, I'd be inclined to add

free(configdir);

return true;
+
+fail:
+ free(configdir);
+
+ return false;
}

and then s/return false/goto fail/ throughout, so as to avoid
duplicating the free() calls. It's a minor point as things stand,
but if more cleanup gets added to the function I think it'd be
easier to maintain this way.

> Sorry for little details in the first email - it's 2:30 AM in my time zone :)

No trouble.

regards, tom lane