bkietz commented on code in PR #46017:
URL: https://github.com/apache/arrow/pull/46017#discussion_r2078224319


##########
cpp/src/parquet/encryption/encryption.cc:
##########
@@ -182,79 +175,75 @@ 
FileEncryptionProperties::Builder::disable_aad_prefix_storage() {
 }
 
 ColumnEncryptionProperties::ColumnEncryptionProperties(bool encrypted,
-                                                       const std::string& 
column_path,
-                                                       const std::string& key,
-                                                       const std::string& 
key_metadata)
-    : column_path_(column_path) {
-  DCHECK(!column_path.empty());
+                                                       std::string column_path,
+                                                       
encryption::SecureString key,
+                                                       std::string 
key_metadata)
+    : column_path_(std::move(column_path)),

Review Comment:
   Member initializer order is asserted to match member declaration order in a 
lint pass. (Member initialization proceeds in declaration order regardless of 
member initializer order.) If the member declarations and initializers were 
both reordered, then
   ```c++
   key_(std::move(key)),
   encrypted_with_footer_key_(encrypted && key.empty()),
   ```
   is use-after-move which should fail a separate lint check. Furthermore, that 
could be fixed by writing
   ```c++
   key_(std::move(key)),
   encrypted_with_footer_key_(encrypted && key_.empty()),
   ```
   
   So I don't think it's necessary to revert.



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