Additional minor pg_dump cleanups
Re-reading Nathans recent 8213df9effaf I noticed a few more small things which
can be cleaned up. In two of the get<Object> functions we lack a fast-path for
when no tuples are found which leads to pg_malloc(0) calls. Another thing is
that we in one place reset the PQExpBuffer immediately after creating it which
isn't required.
--
Daniel Gustafsson
Attachments:
0001-Remove-superfluous-PQExpBuffer-resetting.patchapplication/octet-stream; name=0001-Remove-superfluous-PQExpBuffer-resetting.patch; x-unix-mode=0644Download+0-3
0002-Add-fastpaths-for-when-no-objects-are-found.patchapplication/octet-stream; name=0002-Add-fastpaths-for-when-no-objects-are-found.patch; x-unix-mode=0644Download+8-1
Em qua., 3 de jul. de 2024 às 04:37, Daniel Gustafsson <daniel@yesql.se>
escreveu:
Re-reading Nathans recent 8213df9effaf I noticed a few more small things
which
can be cleaned up. In two of the get<Object> functions we lack a
fast-path for
when no tuples are found which leads to pg_malloc(0) calls. Another thing
is
that we in one place reset the PQExpBuffer immediately after creating it
which
isn't required.
0001 Looks good to me.
0002:
With the function *getPublications* I think it would be good to free up the
allocated memory?
}
+ pg_free(pubinfo);
+cleanup:
PQclear(res);With the function *getExtensions* I think it would be good to return NULL
in case ntups = 0?
Otherwise we may end up with an uninitialized variable.
- ExtensionInfo *extinfo;
+ ExtensionInfo *extinfo = NULL;Funny, the function *getExtensionMembership* does not use the parameter
ExtensionInfo extinfo.
getExtensions does not have another caller, Is it really necessary?
best regards,
Ranier Vilela
On 3 Jul 2024, at 13:29, Ranier Vilela <ranier.vf@gmail.com> wrote:
With the function *getPublications* I think it would be good to free up the allocated memory?
} + pg_free(pubinfo); +cleanup: PQclear(res);
Since the pubinfo is recorded in the DumpableObject and is responsible for
keeping track of which publications to dump, it would be quite incorrect to
free it here.
With the function *getExtensions* I think it would be good to return NULL in case ntups = 0?
Otherwise we may end up with an uninitialized variable.- ExtensionInfo *extinfo; + ExtensionInfo *extinfo = NULL;
I guess that won't hurt, though any code inspecting extinfo when numExtensions
is returned as zero is flat-out wrong. It may however silence a static
analyzer so there is that.
Funny, the function *getExtensionMembership* does not use the parameter ExtensionInfo extinfo.
getExtensions does not have another caller, Is it really necessary?
Yes, see processExtensionTables().
--
Daniel Gustafsson
Attachments:
v2-0002-Add-fastpaths-for-when-no-objects-are-found.patchapplication/octet-stream; name=v2-0002-Add-fastpaths-for-when-no-objects-are-found.patch; x-unix-mode=0644Download+9-2
v2-0001-Remove-superfluous-PQExpBuffer-resetting.patchapplication/octet-stream; name=v2-0001-Remove-superfluous-PQExpBuffer-resetting.patch; x-unix-mode=0644Download+0-3
Em qui., 4 de jul. de 2024 às 05:18, Daniel Gustafsson <daniel@yesql.se>
escreveu:
On 3 Jul 2024, at 13:29, Ranier Vilela <ranier.vf@gmail.com> wrote:
With the function *getPublications* I think it would be good to free up
the allocated memory?
} + pg_free(pubinfo); +cleanup: PQclear(res);Since the pubinfo is recorded in the DumpableObject and is responsible for
keeping track of which publications to dump, it would be quite incorrect to
free it here.With the function *getExtensions* I think it would be good to return
NULL in case ntups = 0?
Otherwise we may end up with an uninitialized variable.
- ExtensionInfo *extinfo; + ExtensionInfo *extinfo = NULL;I guess that won't hurt, though any code inspecting extinfo when
numExtensions
is returned as zero is flat-out wrong. It may however silence a static
analyzer so there is that.Funny, the function *getExtensionMembership* does not use the parameter
ExtensionInfo extinfo.
getExtensions does not have another caller, Is it really necessary?
Yes, see processExtensionTables().
I saw, thanks.
LGTM.
best regards,
Ranier Vilela