Skip to content

Commit

Permalink
fix(r/adbcdrivermanager): Ensure that class of object is checked befo…
Browse files Browse the repository at this point in the history
…re calling R_ExternalPtrAddrFn (#1989)

In the development version of R, calling `R_ExternalPtrAddrFn()` on an
SEXP that is not an external pointer will raise an error. Because we
test the error message, the order of our check vs. the call to
`R_ExternalPtrAddrFn()` matters. In general, it makes more sense to do
our own check first anyway!


https://cran.r-project.org/web/checks/check_results_adbcdrivermanager.html

```
   > test_check("adbcdrivermanager")
    [ FAIL 1 | WARN 0 | SKIP 3 | PASS 176 ]
    
    ══ Skipped tests (3) ═══════════════════════════════════════════════════════════
    • On CRAN (3): 'test-driver_log.R:19:3', 'test-helpers.R:22:3',
      'test-helpers.R:112:3'
    
    ══ Failed tests ════════════════════════════════════════════════════════════════
    ── Error ('test-radbc.R:188:3'): invalid external pointer inputs generate errors ──
    Error in `adbc_database_init_default(driver, list(...))`: R_ExternalPtrAddrFn: argument of type STRSXP is not an external pointer
```
  • Loading branch information
paleolimbot authored Jul 9, 2024
1 parent a1dfefa commit 7dcb2d1
Showing 1 changed file with 6 additions and 4 deletions.
10 changes: 6 additions & 4 deletions r/adbcdrivermanager/src/radbc.cc
Original file line number Diff line number Diff line change
Expand Up @@ -112,12 +112,13 @@ extern "C" SEXP RAdbcLoadDriver(SEXP driver_name_sexp, SEXP entrypoint_sexp) {
}

extern "C" SEXP RAdbcLoadDriverFromInitFunc(SEXP driver_init_func_xptr) {
auto driver_init_func =
reinterpret_cast<AdbcDriverInitFunc>(R_ExternalPtrAddrFn(driver_init_func_xptr));
if (!Rf_inherits(driver_init_func_xptr, "adbc_driver_init_func")) {
Rf_error("Expected external pointer with class '%s'", "adbc_driver_init_func");
}

auto driver_init_func =
reinterpret_cast<AdbcDriverInitFunc>(R_ExternalPtrAddrFn(driver_init_func_xptr));

SEXP driver_xptr = PROTECT(adbc_allocate_xptr<AdbcDriver>());
R_RegisterCFinalizer(driver_xptr, &finalize_driver_xptr);
auto driver = adbc_from_xptr<AdbcDriver>(driver_xptr);
Expand Down Expand Up @@ -148,12 +149,13 @@ extern "C" SEXP RAdbcDatabaseNew(SEXP driver_init_func_xptr) {
adbc_error_stop(status, &error);

if (driver_init_func_xptr != R_NilValue) {
auto driver_init_func =
reinterpret_cast<AdbcDriverInitFunc>(R_ExternalPtrAddrFn(driver_init_func_xptr));
if (!Rf_inherits(driver_init_func_xptr, "adbc_driver_init_func")) {
Rf_error("Expected external pointer with class '%s'", "adbc_driver_init_func");
}

auto driver_init_func =
reinterpret_cast<AdbcDriverInitFunc>(R_ExternalPtrAddrFn(driver_init_func_xptr));

status = AdbcDriverManagerDatabaseSetInitFunc(database, driver_init_func, &error);
adbc_error_stop(status, &error);
}
Expand Down

0 comments on commit 7dcb2d1

Please sign in to comment.