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-adbc.git


The following commit(s) were added to refs/heads/main by this push:
     new 75e49c7f fix(r/adbcdrivermanager): Fix valgrind errors identified by 
CRAN 0.7.0 submission (#1136)
75e49c7f is described below

commit 75e49c7f975e84f2ac3a783bb3d8c7e3645f559b
Author: Dewey Dunnington <[email protected]>
AuthorDate: Fri Sep 29 16:36:13 2023 -0300

    fix(r/adbcdrivermanager): Fix valgrind errors identified by CRAN 0.7.0 
submission (#1136)
    
    There were two problems:
    
    - I forgot that `allocate_xptr<>()` didn't register a finalizer!
    - Apparently `ArrowError`s have to be zero-initialized before getting
    passed to the driver manager to avoid errors.
    
    Closes #1124.
---
 r/adbcdrivermanager/src/radbc.cc | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/r/adbcdrivermanager/src/radbc.cc b/r/adbcdrivermanager/src/radbc.cc
index 5d080cbd..725da7f2 100644
--- a/r/adbcdrivermanager/src/radbc.cc
+++ b/r/adbcdrivermanager/src/radbc.cc
@@ -19,6 +19,8 @@
 #include <R.h>
 #include <Rinternals.h>
 
+#include <string.h>
+
 #include <adbc.h>
 #include "adbc_driver_manager.h"
 
@@ -52,6 +54,7 @@ static void finalize_driver_xptr(SEXP driver_xptr) {
 
   if (driver->release != nullptr) {
     AdbcError error;
+    memset(&error, 0, sizeof(AdbcError));
     int status = driver->release(driver, &error);
     adbc_error_warn(status, &error, "finalize_driver_xptr()");
   }
@@ -68,6 +71,7 @@ static void finalize_database_xptr(SEXP database_xptr) {
 
   if (database->private_data != nullptr) {
     AdbcError error;
+    memset(&error, 0, sizeof(AdbcError));
     int status = AdbcDatabaseRelease(database, &error);
     adbc_error_warn(status, &error, "finalize_database_xptr()");
   }
@@ -83,6 +87,7 @@ extern "C" SEXP RAdbcLoadDriver(SEXP driver_name_sexp, SEXP 
entrypoint_sexp) {
   auto driver = adbc_from_xptr<AdbcDriver>(driver_xptr);
 
   AdbcError error;
+  memset(&error, 0, sizeof(AdbcError));
   int status =
       AdbcLoadDriver(driver_name, entrypoint, ADBC_VERSION_1_0_0, driver, 
&error);
   adbc_error_stop(status, &error, "RAdbcLoadDriver()");
@@ -103,6 +108,7 @@ extern "C" SEXP RAdbcLoadDriverFromInitFunc(SEXP 
driver_init_func_xptr) {
   auto driver = adbc_from_xptr<AdbcDriver>(driver_xptr);
 
   AdbcError error;
+  memset(&error, 0, sizeof(AdbcError));
   int status =
       AdbcLoadDriverFromInitFunc(driver_init_func, ADBC_VERSION_1_0_0, driver, 
&error);
   adbc_error_stop(status, &error, "RAdbcLoadDriverFromInitFunc()");
@@ -118,6 +124,7 @@ extern "C" SEXP RAdbcDatabaseNew(SEXP 
driver_init_func_xptr) {
   AdbcDatabase* database = adbc_from_xptr<AdbcDatabase>(database_xptr);
 
   AdbcError error;
+  memset(&error, 0, sizeof(AdbcError));
   int status = AdbcDatabaseNew(database, &error);
   adbc_error_stop(status, &error, "RAdbcDatabaseNew()");
 
@@ -139,6 +146,7 @@ extern "C" SEXP RAdbcDatabaseNew(SEXP 
driver_init_func_xptr) {
 extern "C" SEXP RAdbcMoveDatabase(SEXP database_xptr) {
   AdbcDatabase* database = adbc_from_xptr<AdbcDatabase>(database_xptr);
   SEXP database_xptr_new = PROTECT(adbc_allocate_xptr<AdbcDatabase>());
+  R_RegisterCFinalizer(database_xptr_new, &finalize_database_xptr);
   AdbcDatabase* database_new = adbc_from_xptr<AdbcDatabase>(database_xptr_new);
 
   memcpy(database_new, database, sizeof(AdbcDatabase));
@@ -184,6 +192,7 @@ static void finalize_connection_xptr(SEXP connection_xptr) {
 
   if (connection->private_data != nullptr) {
     AdbcError error;
+    memset(&error, 0, sizeof(AdbcError));
     int status = AdbcConnectionRelease(connection, &error);
     adbc_error_warn(status, &error, "finalize_connection_xptr()");
   }
@@ -198,6 +207,7 @@ extern "C" SEXP RAdbcConnectionNew(void) {
   AdbcConnection* connection = adbc_from_xptr<AdbcConnection>(connection_xptr);
 
   AdbcError error;
+  memset(&error, 0, sizeof(AdbcError));
   int status = AdbcConnectionNew(connection, &error);
   adbc_error_stop(status, &error, "RAdbcConnectionNew()");
 
@@ -208,6 +218,7 @@ extern "C" SEXP RAdbcConnectionNew(void) {
 extern "C" SEXP RAdbcMoveConnection(SEXP connection_xptr) {
   AdbcConnection* connection = adbc_from_xptr<AdbcConnection>(connection_xptr);
   SEXP connection_xptr_new = PROTECT(adbc_allocate_xptr<AdbcConnection>());
+  R_RegisterCFinalizer(connection_xptr_new, &finalize_connection_xptr);
   AdbcConnection* connection_new = 
adbc_from_xptr<AdbcConnection>(connection_xptr_new);
 
   memcpy(connection_new, connection, sizeof(AdbcConnection));
@@ -369,6 +380,7 @@ static void finalize_statement_xptr(SEXP statement_xptr) {
 
   if (statement->private_data != nullptr) {
     AdbcError error;
+    memset(&error, 0, sizeof(AdbcError));
     int status = AdbcStatementRelease(statement, &error);
     adbc_error_warn(status, &error, "finalize_statement_xptr()");
   }
@@ -384,6 +396,7 @@ extern "C" SEXP RAdbcStatementNew(SEXP connection_xptr) {
   AdbcStatement* statement = adbc_from_xptr<AdbcStatement>(statement_xptr);
 
   AdbcError error;
+  memset(&error, 0, sizeof(AdbcError));
   int status = AdbcStatementNew(connection, statement, &error);
   adbc_error_stop(status, &error, "RAdbcStatementNew()");
 
@@ -396,6 +409,7 @@ extern "C" SEXP RAdbcStatementNew(SEXP connection_xptr) {
 extern "C" SEXP RAdbcMoveStatement(SEXP statement_xptr) {
   AdbcStatement* statement = adbc_from_xptr<AdbcStatement>(statement_xptr);
   SEXP statement_xptr_new = PROTECT(adbc_allocate_xptr<AdbcStatement>());
+  R_RegisterCFinalizer(statement_xptr_new, &finalize_statement_xptr);
   AdbcStatement* statement_new = 
adbc_from_xptr<AdbcStatement>(statement_xptr_new);
 
   memcpy(statement_new, statement, sizeof(AdbcStatement));

Reply via email to