Some review feedback

Author: rok

cpp/CMakeLists.txt

@@ -729,7 +729,8 @@ install(FILES ${CMAKE_CURRENT_SOURCE_DIR}/gdb_arrow.py DESTINATION "${ARROW_GDB_
 install(DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR}/src/generated/
         DESTINATION "${CMAKE_INSTALL_INCLUDEDIR}/generated"
         FILES_MATCHING
-        PATTERN "*.h")
+        PATTERN "*.h"
+        PATTERN "*.tcc")
 
 #
 # Validate and print out Arrow configuration options

cpp/src/parquet/CMakeLists.txt

@@ -309,9 +309,9 @@ add_arrow_lib(parquet
               STATIC_LINK_LIBS
               ${PARQUET_STATIC_LINK_LIBS}
               STATIC_INSTALL_INTERFACE_LIBS
-              ${PARQUET_STATIC_INSTALL_INTERFACE_LIBS})
-
-target_include_directories(parquet_objlib SYSTEM PRIVATE ${ARROW_SOURCE_DIR}/thirdparty/flatbuffers/include)
+              ${PARQUET_STATIC_INSTALL_INTERFACE_LIBS}
+              PRIVATE_INCLUDES
+              ${ARROW_SOURCE_DIR}/thirdparty/flatbuffers/include)
 
 if(WIN32 AND NOT (ARROW_TEST_LINKAGE STREQUAL "static"))
   add_library(parquet_test_support STATIC

cpp/src/parquet/file_reader.cc

@@ -441,13 +441,14 @@ class SerializedFile : public ParquetFileReader::Contents {
     PARQUET_ASSIGN_OR_THROW(
         auto footer_buffer,
         source_->ReadAt(source_size_ - footer_read_size, footer_read_size));
-    if (properties_.read_metadata3()) {
+    if (properties_.read_flatbuffer_metadata_if_present()) {
       // Try to extract flatbuffer metadata from footer
       std::string flatbuffer_data;
       auto result = ExtractFlatbuffer(footer_buffer, &flatbuffer_data);
       if (result.ok()) {
-        int32_t required_or_consumed = *result;
-        if (required_or_consumed > static_cast<int32_t>(footer_buffer->size())) {
+        uint32_t required_or_consumed = *result;
+        if (required_or_consumed > static_cast<uint32_t>(footer_buffer->size()) &&
+            static_cast<int64_t>(required_or_consumed) <= source_size_) {
           PARQUET_ASSIGN_OR_THROW(
               footer_buffer,
               source_->ReadAt(source_size_ - required_or_consumed, required_or_consumed));
@@ -461,8 +462,8 @@ class SerializedFile : public ParquetFileReader::Contents {
               format3::GetFileMetaData(flatbuffer_data.data());
           auto thrift_metadata =
               std::make_unique<format::FileMetaData>(FromFlatbuffer(fb_metadata));
-          file_metadata_ = FileMetaData::Make(
-              std::move(thrift_metadata), static_cast<uint32_t>(*result), properties_);
+          file_metadata_ =
+              FileMetaData::Make(std::move(thrift_metadata), *result, properties_);
           return;
         }
       }

cpp/src/parquet/file_writer.h

@@ -110,8 +110,7 @@ class PARQUET_EXPORT RowGroupWriter {
 };
 
 PARQUET_EXPORT
-void WriteFileMetaData(const FileMetaData& file_metadata,
-                       ::arrow::io::OutputStream* sink,
+void WriteFileMetaData(const FileMetaData& file_metadata, ::arrow::io::OutputStream* sink,
                        bool use_metadata3 = false);
 
 PARQUET_EXPORT

cpp/src/parquet/metadata.h

@@ -30,7 +30,10 @@
 #include "parquet/platform.h"
 #include "parquet/properties.h"
 #include "parquet/type_fwd.h"
-#include "generated/parquet_types.h"
+
+namespace parquet::format {
+class FileMetaData;
+}
 
 namespace parquet {
 
@@ -389,8 +392,8 @@ class PARQUET_EXPORT FileMetaData {
                         const ReaderProperties& properties,
                         std::shared_ptr<InternalFileDecryptor> file_decryptor = NULLPTR);
 
-  explicit FileMetaData(std::unique_ptr<format::FileMetaData> metadata, uint32_t metadata_len,
-                        const ReaderProperties& properties);
+  explicit FileMetaData(std::unique_ptr<format::FileMetaData> metadata,
+                        uint32_t metadata_len, const ReaderProperties& properties);
 
   void set_file_decryptor(std::shared_ptr<InternalFileDecryptor> file_decryptor);
   const std::shared_ptr<InternalFileDecryptor>& file_decryptor() const;