fenfeng9 commented on code in PR #50768:
URL: https://github.com/apache/arrow/pull/50768#discussion_r4124925772
##########
cpp/gdb_arrow.py:
##########
@@ -769,7 +772,8 @@ def __getitem__(self, index):
def from_buffer(cls, buf, offset, length):
assert isinstance(buf, Buffer)
byte_offset, bit_offset = divmod(offset, 8)
- byte_length = math.ceil(length + offset / 8) - byte_offset
+ # E.g. offset=3, length=6 selects bits 3..8 and needs 2 bytes.
+ byte_length = math.ceil((bit_offset + length) / 8)
Review Comment:
The existing BooleanArray slice tests already cover this case:
```cpp
auto heap_bool_array_sliced_1_9 =
SliceArrayFromJSON(boolean(), json_bool_array, 1, 9);
auto heap_bool_array_sliced_2_6 =
SliceArrayFromJSON(boolean(), json_bool_array, 2, 6);
```
They exercise `Bitmap.from_buffer()` with non-byte-aligned offsets and
ranges that cross byte boundaries.
Before this change, `Buffer.bytes_view()` ignored the requested length:
```python
if length is None:
length = self.size
mem = gdb.selected_inferior().read_memory(
self.val['data_'] + offset, self.size)
```
It always read `self.size` bytes. Now it reads the requested length:
```python
mem = gdb.selected_inferior().read_memory(
self.val['data_'] + offset, length)
```
This previously masked the incorrect byte-length calculation in
Bitmap.from_buffer(). Since the existing tests already cover this path, I did
not add another test case.
--
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]
