Re: Sloppiness around failure handling of parsePGArray in pg_dump

> On 19 Nov 2020, at 02:37, Michael Paquier <michael(at)paquier(dot)xyz> wrote:
>
> On Wed, Nov 18, 2020 at 10:19:40AM +0100, Daniel Gustafsson wrote:
>> I agree that we should fix this even if it will have quite limited impact in
>> production settings. Patch LGTM, +1.
>
> Thanks. I have reviewed that again this morning and applied it.
>
>> Another thing caught my eye here (while not the fault of this patch), we ensure
>> to clean up array leftover in case of parsePGArray failure, but we don't clean
>> up the potential allocations from the previous calls. Something like the below
>> seems more consistent.
>>
>> @@ -12105,6 +12099,8 @@ dumpFunc(Archive *fout, FuncInfo *finfo)
>> nitems != nallargs)
>> {
>> pg_log_warning("could not parse proargmodes array");
>> + if (allargtypes)
>> + free(allargtypes);
>> if (argmodes)
>> free(argmodes);
>> argmodes = NULL;
>> @@ -12119,6 +12115,10 @@ dumpFunc(Archive *fout, FuncInfo *finfo)
>> nitems != nallargs)
>> {
>> pg_log_warning("could not parse proargnames array");
>> + if (allargtypes)
>> + free(allargtypes);
>> + if (argmodes)
>> + free(argmodes);
>> if (argnames)
>> free(argnames);
>> argnames = NULL;
>
> If you do that, I think that's not completely correct either.
> format_function_arguments_old() has some logic to allow the process to
> go through for pre-8.4 dumps as long as allargtypes includes correct

Ah, yes, I read that wrong. It's correct as it is.

cheers ./daniel