Re: [PATCH] Redudant initilization - Mailing list pgsql-hackers
From | Ranier Vilela |
---|---|
Subject | Re: [PATCH] Redudant initilization |
Date | |
Msg-id | CAEudQApokvHWVG5Sjt3CV68FGpRLqG==hG5_tzmXo9dF3w-5ww@mail.gmail.com Whole thread Raw |
In response to | Re: [PATCH] Redudant initilization (Kyotaro Horiguchi <horikyota.ntt@gmail.com>) |
Responses |
Re: [PATCH] Redudant initilization
|
List | pgsql-hackers |
Em dom., 29 de mar. de 2020 às 21:57, Kyotaro Horiguchi <horikyota.ntt@gmail.com> escreveu:
Hello.
At Sat, 28 Mar 2020 19:04:00 -0300, Ranier Vilela <ranier.vf@gmail.com> wrote in
> Hi,
> This patch fixes some redundant initilization, that are safe to remove.
> diff --git a/src/backend/access/gist/gistxlog.c b/src/backend/access/gist/gistxlog.c
> index d3f3a7b803..ffaa2b1ab4 100644
> --- a/src/backend/access/gist/gistxlog.c
> +++ b/src/backend/access/gist/gistxlog.c
> @@ -396,7 +396,7 @@ gistRedoPageReuse(XLogReaderState *record)
> if (InHotStandby)
> {
> FullTransactionId latestRemovedFullXid = xlrec->latestRemovedFullXid;
> - FullTransactionId nextFullXid = ReadNextFullTransactionId();
> + FullTransactionId nextFullXid;
I'd prefer to preserve this and remove the latter.
Ok.
> diff --git a/src/backend/catalog/heap.c b/src/backend/catalog/heap.c
> index 9d9e915979..795cf349eb 100644
> --- a/src/backend/catalog/heap.c
> +++ b/src/backend/catalog/heap.c
> @@ -3396,7 +3396,7 @@ List *
> heap_truncate_find_FKs(List *relationIds)
> {
> List *result = NIL;
> - List *oids = list_copy(relationIds);
> + List *oids;
This was just a waste of memory, the fix looks fine.
> diff --git a/src/backend/storage/smgr/md.c b/src/backend/storage/smgr/md.c
> index c5b771c531..37fbeef841 100644
> --- a/src/backend/storage/smgr/md.c
> +++ b/src/backend/storage/smgr/md.c
> @@ -730,9 +730,11 @@ mdwrite(SMgrRelation reln, ForkNumber forknum, BlockNumber blocknum,
> BlockNumber
> mdnblocks(SMgrRelation reln, ForkNumber forknum)
> {
> - MdfdVec *v = mdopenfork(reln, forknum, EXTENSION_FAIL);
> + MdfdVec *v;
> BlockNumber nblocks;
> - BlockNumber segno = 0;
> + BlockNumber segno;
> +
> + mdopenfork(reln, forknum, EXTENSION_FAIL);
>
> /* mdopen has opened the first segment */
> Assert(reln->md_num_open_segs[forknum] > 0);
It doesn't seems *to me* an issue.
Not a big deal, but the assignment of the variable v here is a small waste, since it is again highlighted right after.
> diff --git a/src/backend/utils/adt/json.c b/src/backend/utils/adt/json.c
> index 8bb00abb6b..7a6a2ecbe9 100644
> --- a/src/backend/utils/adt/json.c
> +++ b/src/backend/utils/adt/json.c
> @@ -990,7 +990,7 @@ catenate_stringinfo_string(StringInfo buffer, const char *addon)
> Datum
> json_build_object(PG_FUNCTION_ARGS)
> {
> - int nargs = PG_NARGS();
> + int nargs;
This part looks fine.
> int i;
> const char *sep = "";
> StringInfo result;
> @@ -998,6 +998,8 @@ json_build_object(PG_FUNCTION_ARGS)
> bool *nulls;
> Oid *types;
>
> + PG_NARGS();
> +
> /* fetch argument values to build the object */
> nargs = extract_variadic_args(fcinfo, 0, false, &args, &types, &nulls);
PG_NARGS() doesn't have a side-effect so no need to call independently.
Sorry, does that mean we can remove it completely?
> diff --git a/src/backend/utils/mmgr/mcxt.c b/src/backend/utils/mmgr/mcxt.c
> index 9e24fec72d..fb0e833b2d 100644
> --- a/src/backend/utils/mmgr/mcxt.c
> +++ b/src/backend/utils/mmgr/mcxt.c
> @@ -475,7 +475,7 @@ MemoryContextMemAllocated(MemoryContext context, bool recurse)
>
> if (recurse)
> {
> - MemoryContext child = context->firstchild;
> + MemoryContext child;
>
> for (child = context->firstchild;
> child != NULL;
This looks fine.
Thank you for the review and consideration.
best regards,
Ranier Vilela
pgsql-hackers by date: