To be pedantic. That code is just whatever was in .99.23.1 which we inherited from. I'm fine with fixing the code from Christian's patch. The zcalloc is going to do the right thing.
All in all this isn't worth arguing to much about it. After all it's memory code. It shouldn't change much in the future :) donald On Tue, May 3, 2016 at 5:10 PM, Lou Berger <[email protected]> wrote: > just looked at the cumulus code. The only effective difference is in > allocation failure reporting. My change has a failure in realloc with > a null reported as a "calloc" error while the cumulus will report it as > a "realloc" error. > > I think either version is fine. > > Lou > > On 5/3/2016 4:53 PM, Lou Berger wrote: > > Hi Donald, Christian, > > > > On 5/3/2016 2:06 PM, Donald Sharp wrote: > >> Well Lou added the code. The commit log referenced some memory issue > >> that was found that this fixes. Might be worthwhile getting some > >> feedback on that. > >> > > The issue was seen in Martin's tests on our bgp safi changes that went > > into the last release. I'd need to dig a bit to figure out which test > > was failing. The change brought the call to be in line with the > > standard (posix, etc.) realloc call. > > > > Christian, is right that the change resulted in rendering later code > > irrelevant. > > > >> I'm not sure we should go carte-blanch back to what we have :) > >> > > Well this is one way to rediscover which of Martin's test failed ;-) > > > > Lou > >> donald > >> > >> On Tue, May 3, 2016 at 2:03 PM, Christian Franke > >> <[email protected] <mailto:[email protected]>> > wrote: > >> > >> On 05/03/2016 07:46 PM, Donald Sharp wrote: > >> > You are right for the upstream behavior. The version that we > >> have in > >> > the cumulus tree is this: > >> > > >> > 106 void * > >> > 107 zrealloc (int type, void *ptr, size_t size) > >> > 108 { > >> > 109 void *memory; > >> > 110 > >> > 111 memory = realloc (ptr, size); > >> > 112 if (memory == NULL) > >> > 113 zerror ("realloc", type, size); > >> > 114 if (ptr == NULL) > >> > 115 alloc_inc (type); > >> > 116 > >> > 117 return memory; > >> > 118 } > >> > > >> > > >> > Which would not work too well with your patch :) > >> > > >> > ah well. My mistake. > >> > > >> > acked-by: Donald Sharp <[email protected] > >> <mailto:[email protected]> > >> > <mailto:[email protected] > >> <mailto:[email protected]>>> > >> > >> Then let's stick with your version. Imho, either is fine and it's > >> probably good to keep merge conflicts low. > >> > >> -Christian > >> > >> > > > > > > _______________________________________________ > > Quagga-dev mailing list > > [email protected] > > https://lists.quagga.net/mailman/listinfo/quagga-dev > > > > >
_______________________________________________ Quagga-dev mailing list [email protected] https://lists.quagga.net/mailman/listinfo/quagga-dev
