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