felipecrv commented on code in PR #36431:
URL: https://github.com/apache/arrow/pull/36431#discussion_r1252261920


##########
cpp/src/arrow/array/util.cc:
##########
@@ -296,11 +301,14 @@ class ArrayDataEndianSwapper {
 namespace internal {
 
 Result<std::shared_ptr<ArrayData>> SwapEndianArrayData(
-    const std::shared_ptr<ArrayData>& data) {
+    const std::shared_ptr<ArrayData>& data, MemoryPool* pool) {
   if (data->offset != 0) {
     return Status::Invalid("Unsupported data format: data.offset != 0");
   }
-  ArrayDataEndianSwapper swapper(data);
+  if (pool == nullptr) {
+    return Status::Invalid("Unsupported pool == nullptr");
+  }

Review Comment:
   I think a `DCHECK(pool)` is enough here.
   
   The reason we pass `*` instead of `&` in functions like this is that we 
follow the Google Style convention that says non-const "reference" parameters 
should be pointers instead of `&`. A consequence of that convention is that we 
expect the caller to do the right thing and pass a non-null mutable argument 
unless stated in the function documentation that `nullptr` is allowed.
   



-- 
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]

Reply via email to