You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@arrow.apache.org by GitBox <gi...@apache.org> on 2022/08/11 20:52:35 UTC

[GitHub] [arrow-nanoarrow] lidavidm commented on a diff in pull request #19: ArrowArray consumer buffer helpers

lidavidm commented on code in PR #19:
URL: https://github.com/apache/arrow-nanoarrow/pull/19#discussion_r943929415


##########
src/nanoarrow/utils_inline.h:
##########
@@ -26,6 +26,115 @@
 extern "C" {
 #endif
 
+static inline void ArrowLayoutInit(struct ArrowLayout* layout,

Review Comment:
   Just an observation, but I think this sort of function doesn't necessarily have to be inline - it shouldn't be called frequently and shouldn't be a bottleneck



##########
src/nanoarrow/utils_inline.h:
##########
@@ -26,6 +26,115 @@
 extern "C" {
 #endif
 
+static inline void ArrowLayoutInit(struct ArrowLayout* layout,

Review Comment:
   Or, of course, you could go the other way and put _everything_ in headers, making this a header-only library



##########
src/nanoarrow/typedefs_inline.h:
##########
@@ -179,6 +217,19 @@ struct ArrowStringView {
   int64_t n_bytes;
 };
 
+/// \brief An non-owning view of a buffer
+struct ArrowBufferView {
+  /// \brief A pointer to the start of the buffer
+  ///
+  /// If n_bytes is 0, this value may be NULL.
+  const union ArrowBufferDataPointer data;
+
+  /// \brief The size of the string in bytes,
+  ///
+  /// (Not including the null terminator.)
+  int64_t n_bytes;

Review Comment:
   ```suggestion
     /// \brief The size of the buffer in bytes.
     int64_t n_bytes;
   ```



-- 
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: issues-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org