This is an automated email from the ASF dual-hosted git repository.
paleolimbot pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/arrow-nanoarrow.git
The following commit(s) were added to refs/heads/main by this push:
new ffdd70c3 chore: Help clang-tidy avoid errors when calling
AppendBitmapInt8 (#751)
ffdd70c3 is described below
commit ffdd70c3b951890d7728070f48062abe5c83c8c6
Author: Dewey Dunnington <de...@dunnington.ca>
AuthorDate: Sat May 3 14:27:51 2025 -0500
chore: Help clang-tidy avoid errors when calling AppendBitmapInt8 (#751)
I tried to add a test that would trigger the sanitizer error based on
https://github.com/vyasr/nanoarrow_clang_tidy_error ; however, I think
clang-tidy is too smart when it's all integrated in the same project and
I don't see the error appear.
Closes #662.
---
src/nanoarrow/common/inline_buffer.h | 6 ++++++
src/nanoarrow/ipc/reader_test.cc | 15 ++++++++++++++-
src/nanoarrow/ipc/writer_test.cc | 14 ++++++++++++++
3 files changed, 34 insertions(+), 1 deletion(-)
diff --git a/src/nanoarrow/common/inline_buffer.h
b/src/nanoarrow/common/inline_buffer.h
index e26c2de9..c9a78602 100644
--- a/src/nanoarrow/common/inline_buffer.h
+++ b/src/nanoarrow/common/inline_buffer.h
@@ -624,6 +624,9 @@ static inline void ArrowBitmapAppendInt8Unsafe(struct
ArrowBitmap* bitmap,
return;
}
+ NANOARROW_DCHECK(bitmap->buffer.data != NULL);
+ NANOARROW_DCHECK(values != NULL);
+
const int8_t* values_cursor = values;
int64_t n_remaining = n_values;
int64_t out_i_cursor = bitmap->size_bits;
@@ -671,6 +674,9 @@ static inline void ArrowBitmapAppendInt32Unsafe(struct
ArrowBitmap* bitmap,
return;
}
+ NANOARROW_DCHECK(bitmap->buffer.data != NULL);
+ NANOARROW_DCHECK(values != NULL);
+
const int32_t* values_cursor = values;
int64_t n_remaining = n_values;
int64_t out_i_cursor = bitmap->size_bits;
diff --git a/src/nanoarrow/ipc/reader_test.cc b/src/nanoarrow/ipc/reader_test.cc
index 9dda66fd..cd6374a6 100644
--- a/src/nanoarrow/ipc/reader_test.cc
+++ b/src/nanoarrow/ipc/reader_test.cc
@@ -99,6 +99,16 @@ TEST(NanoarrowIpcReader, InputStreamBuffer) {
stream.release(&stream);
}
+// clang-tidy helpfully reminds us that file_ptr might not be released
+// if an assertion fails
+struct FileCloser {
+ FileCloser(FILE* file) : file_(file) {}
+ ~FileCloser() {
+ if (file_) fclose(file_);
+ }
+ FILE* file_{};
+};
+
TEST(NanoarrowIpcReader, InputStreamFile) {
struct ArrowIpcInputStream stream;
errno = EINVAL;
@@ -106,6 +116,7 @@ TEST(NanoarrowIpcReader, InputStreamFile) {
uint8_t input_data[] = {0x01, 0x02, 0x03, 0x04, 0x05};
FILE* file_ptr = tmpfile();
+ FileCloser closer{file_ptr};
ASSERT_NE(file_ptr, nullptr);
ASSERT_EQ(fwrite(input_data, 1, sizeof(input_data), file_ptr),
sizeof(input_data));
fseek(file_ptr, 0, SEEK_SET);
@@ -113,7 +124,9 @@ TEST(NanoarrowIpcReader, InputStreamFile) {
uint8_t output_data[] = {0xff, 0xff, 0xff, 0xff, 0xff};
int64_t size_read_bytes;
- ASSERT_EQ(ArrowIpcInputStreamInitFile(&stream, file_ptr, 1), NANOARROW_OK);
+ ASSERT_EQ(ArrowIpcInputStreamInitFile(&stream, file_ptr,
/*close_on_release=*/1),
+ NANOARROW_OK);
+ closer.file_ = nullptr;
EXPECT_EQ(stream.read(&stream, output_data, 2, &size_read_bytes, nullptr),
NANOARROW_OK);
diff --git a/src/nanoarrow/ipc/writer_test.cc b/src/nanoarrow/ipc/writer_test.cc
index 4cee5618..a07ae516 100644
--- a/src/nanoarrow/ipc/writer_test.cc
+++ b/src/nanoarrow/ipc/writer_test.cc
@@ -51,8 +51,19 @@ TEST(NanoarrowIpcWriter, OutputStreamBuffer) {
header + message + message + message + message);
}
+// clang-tidy helpfully reminds us that file_ptr might not be released
+// if an assertion fails
+struct FileCloser {
+ FileCloser(FILE* file) : file_(file) {}
+ ~FileCloser() {
+ if (file_) fclose(file_);
+ }
+ FILE* file_{};
+};
+
TEST(NanoarrowIpcWriter, OutputStreamFile) {
FILE* file_ptr = tmpfile();
+ FileCloser closer{file_ptr};
ASSERT_NE(file_ptr, nullptr);
// Start by writing some header
@@ -65,6 +76,7 @@ TEST(NanoarrowIpcWriter, OutputStreamFile) {
nanoarrow::ipc::UniqueOutputStream stream;
ASSERT_EQ(ArrowIpcOutputStreamInitFile(stream.get(), file_ptr,
/*close_on_release=*/1),
NANOARROW_OK);
+ closer.file_ = nullptr;
struct ArrowError error;
@@ -98,9 +110,11 @@ TEST(NanoarrowIpcWriter, OutputStreamFileError) {
auto phony_path = __FILE__ + std::string(".phony");
FILE* file_ptr = fopen(phony_path.c_str(), "rb");
+ FileCloser closer{file_ptr};
ASSERT_EQ(file_ptr, nullptr);
EXPECT_EQ(ArrowIpcOutputStreamInitFile(stream.get(), file_ptr,
/*close_on_release=*/1),
ENOENT);
+ closer.file_ = nullptr;
}
struct ArrowIpcWriterPrivate {
