yilin0518 commented on issue #9289: URL: https://github.com/apache/arrow-rs/issues/9289#issuecomment-3809047812
> I _think_ this might be a false positive. Looking at alloc: > > [arrow-rs/arrow-buffer/src/buffer/mutable.rs](https://github.com/apache/arrow-rs/blob/a49af1de8543b844430d799dff89d125a6f87221/arrow-buffer/src/buffer/mutable.rs#L131-L138) > > Lines 131 to 138 in [a49af1d](/apache/arrow-rs/commit/a49af1de8543b844430d799dff89d125a6f87221) > > let data = match layout.size() { > 0 => dangling_ptr(), > _ => { > // Safety: Verified size != 0 > let raw_ptr = unsafe { std::alloc::alloc(layout) }; > NonNull::new(raw_ptr).unwrap_or_else(|| handle_alloc_error(layout)) > } > }; > We have code to call `handle_alloc_error` in case the returned pointer is null, which is returned if memory is exhausted: > > > Returning a null pointer indicates that either memory is exhausted or `layout` does not meet this allocator’s size or alignment constraints. > > * https://doc.rust-lang.org/std/alloc/trait.GlobalAlloc.html#tymethod.alloc > > For reference, the other calls to `alloc` have the same handling: > > [arrow-rs/arrow-buffer/src/buffer/mutable.rs](https://github.com/apache/arrow-rs/blob/a49af1de8543b844430d799dff89d125a6f87221/arrow-buffer/src/buffer/mutable.rs#L163-L167) > > Lines 163 to 167 in [a49af1d](/apache/arrow-rs/commit/a49af1de8543b844430d799dff89d125a6f87221) > > _ => { > // Safety: Verified size != 0 > let raw_ptr = unsafe { std::alloc::alloc_zeroed(layout) }; > NonNull::new(raw_ptr).unwrap_or_else(|| handle_alloc_error(layout)) > } > [arrow-rs/arrow-buffer/src/buffer/mutable.rs](https://github.com/apache/arrow-rs/blob/a49af1de8543b844430d799dff89d125a6f87221/arrow-buffer/src/buffer/mutable.rs#L346-L352) > > Lines 346 to 352 in [a49af1d](/apache/arrow-rs/commit/a49af1de8543b844430d799dff89d125a6f87221) > > let data = match self.layout.size() { > // Safety: new_layout is not empty > 0 => unsafe { std::alloc::alloc(new_layout) }, > // Safety: verified new layout is valid and not empty > _ => unsafe { std::alloc::realloc(self.as_mut_ptr(), self.layout, capacity) }, > }; > self.data = NonNull::new(data).unwrap_or_else(|| handle_alloc_error(new_layout)); So the expected behaviour of allocation failure is program abort? Could you consider that adding a safety comment about the `new_with_len` to remind user that create a new NullBufferBuilder with a very large capacity maybe result in allocation fail? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
