Hi, I misunderstood the issue: the zeroing of the padding is correctly handled by the writers, as long as they are careful not to read beyond the specified size of the buffers (as opposed to its capacity).
The valgrind warning came from the way AppendNull(s) is implemented in some of the builders: it just sets the nullmap and increments lengths, but doesn't set the data, which leads to uninitialized memory chunks, where nulls where inserted. I'll send a PR which addresses it. I'm still not sure if we should add zero the padding anyway, since the buffers can be shared in any number of ways. Zeroing the last couple of bytes shouldn't have any performance impact and could save a headache in the future. Cheers. On Tue, Jul 3, 2018 at 1:15 PM Antoine Pitrou <anto...@python.org> wrote: > > Hi, > > Writing uninitialized memory risks leaking private data. This might > lead to security issues. > > I'd go for option 2. Option 3 sounds much more costly (we would be > zero-initializing large memory areas instead of small padding areas). > > Regards > > Antoine. > > > Le 03/07/2018 à 13:11, Dimitri Vorona a écrit : > > Hi all, > > > > currently, running json-integration-test with valgrind leads to the > > following warning: "Syscall param write(buf) points to uninitialised > > byte(s)". This is caused by PrimitiveBufferBuilder not initializing its > > data memory. Note: we initialize null_bitmap_data_ by zeroing, i.e. > setting > > all values to not-null by default. > > > > Since having tests pass valgrind might be desirable for the CI, I think > we > > should fix this warning. There are a couple of possibilities: > > > > 1. Add suppression. The specs doesn't require padding to have a specific > > value, so we might consider it to be false positive > > 2. Add initialization of the padding bytes to ArrayBuilder::FinishIntenal > > implementations. > > 3. Generally zero-initialize memory in PoolBuffer. Might be too > expensive. > > > > Or course there could be number of other options, includeing "do > nothing". > > If we settle on a best option, I can make a PR. > > > > Cheers, > > Dimitri. > > >
