Re: Clang 3.3 Analyzer Results

Kevin Grittner <kgrittn(at)ymail(dot)com> writes:
> Kevin Grittner <kgrittn(at)ymail(dot)com> wrote:
>> Memory Error
>> Double free: 1
>> Memory leak: 1

> These both seemed legitimate to me. Patch attached. Any
> objections to applying it? I realize the memory leak is a tiny one
> in the regression testing code, so it could never amount to enough
> to matter; but it seems worth fixing just to avoid noise in code
> analyzers.

The logic, if you can call it that, in streamutil.c makes my head hurt.
I think it needs more work than what you did here --- for one thing,
the free(password) call results in values[i] becoming a dangling pointer
to freed memory, and it's not exactly obvious that that pointer will get
overwritten again before it's used.

Moreover, although the apparent intention of the dbpassword static state
is to allow a password to be saved and reused across calls, it kinda looks
like that's broken by the odd choice to make dbgetpassword also static.
At the very least that choice makes it a lot harder to analyze what will
happen in calls after the first.

I think the looping to get a password here should be thrown out and
rewritten from scratch, or at least cribbed from someplace with less
contorted logic.

regards, tom lane