Aleksander Alekseev <aleksander@tigerdata.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
