At Tue, 19 Jul 2022 16:57:14 +0900 (JST), Kyotaro Horiguchi <horikyota....@gmail.com> wrote in > At Tue, 19 Jul 2022 16:02:26 +0900, Masahiko Sawada <sawada.m...@gmail.com> > wrote in > > On Tue, Jul 19, 2022 at 1:47 PM Amit Kapila <amit.kapil...@gmail.com> wrote: > > > Good work. I wonder without comments this may create a problem in the > > > future. OTOH, I don't see adding a check "catchange.xcnt > 0" before > > > freeing the memory any less robust. Also, for consistency, we can use > > > a similar check based on xcnt in the SnapBuildRestore to free the > > > memory in the below code: > > > + /* set catalog modifying transactions */ > > > + if (builder->catchange.xip) > > > + pfree(builder->catchange.xip); > > > > I would hesitate to add comments about preventing the particular > > optimization. I think we do null-pointer-check-then-pfree many place. > > It seems to me that checking the array length before memcpy is more > > natural than checking both the array length and the array existence > > before pfree. > > Anyway according to commit message of 46ab07ffda, POSIX forbits > memcpy(NULL, NULL, 0). It seems to me that it is the cause of the > false (or over) optimization. So if we add some comment, it would be > for memcpy, not pfree..
For clarilty, I meant that I don't think we need that comment. regards. -- Kyotaro Horiguchi NTT Open Source Software Center
