Re: [PATCH] Redudant initilization

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