Skip to content

Commit

Permalink
Another PROTECT(getAttrib(.)), UNPROTECT() early returns
Browse files Browse the repository at this point in the history
  • Loading branch information
MichaelChirico authored Oct 31, 2024
1 parent 0358f99 commit 18ef60e
Show file tree
Hide file tree
Showing 2 changed files with 5 additions and 2 deletions.
5 changes: 4 additions & 1 deletion src/assign.c
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,7 @@ static int _selfrefok(SEXP x, Rboolean checkNames, Rboolean verbose) {
p = R_ExternalPtrAddr(v);
if (p==NULL) {
if (verbose) Rprintf(_(".internal.selfref ptr is NULL. This is expected and normal for a data.table loaded from disk. Please remember to always setDT() immediately after loading to prevent unexpected behavior. If this table was not loaded from disk or you've already run setDT(), please report to data.table issue tracker.\n"));
UNPROTECT(nprotect);
return -1;
}
if (!isNull(p)) internal_error(__func__, ".internal.selfref ptr is neither NULL nor R_NilValue"); // # nocov
Expand All @@ -132,8 +133,10 @@ static int _selfrefok(SEXP x, Rboolean checkNames, Rboolean verbose) {
// R copied this vector not data.table; it's not actually over-allocated. It looks over-allocated
// because R copies the original vector's tl over despite allocating length.
prot = R_ExternalPtrProtected(v);
if (TYPEOF(prot) != EXTPTRSXP) // Very rare. Was error(_(".internal.selfref prot is not itself an extptr")).
if (TYPEOF(prot) != EXTPTRSXP) { // Very rare. Was error(_(".internal.selfref prot is not itself an extptr")).
UNPROTECT(nprotect);

Check warning on line 137 in src/assign.c

View check run for this annotation

Codecov / codecov/patch

src/assign.c#L137

Added line #L137 was not covered by tests
return 0; // # nocov ; see http://stackoverflow.com/questions/15342227/getting-a-random-internal-selfref-error-in-data-table-for-r
}
if (x!=R_ExternalPtrAddr(prot) && !ALTREP(x))
SET_TRUELENGTH(x, LENGTH(x)); // R copied this vector not data.table, it's not actually over-allocated
int ok = checkNames ? names==tag : x==R_ExternalPtrAddr(prot);
Expand Down
2 changes: 1 addition & 1 deletion src/subset.c
Original file line number Diff line number Diff line change
Expand Up @@ -345,7 +345,7 @@ SEXP subsetDT(SEXP x, SEXP rows, SEXP cols) { // API change needs update NEWS.md
// clear any index that was copied over by copyMostAttrib() above, e.g. #1760 and #1734 (test 1678)
setAttrib(ans, sym_index, R_NilValue);
// but maintain key if ordered subset
SEXP key = getAttrib(x, sym_sorted);
SEXP key = PROTECT(getAttrib(x, sym_sorted)); nprotect++;
if (length(key)) {
SEXP ans_names = PROTECT(getAttrib(ans, R_NamesSymbol)); nprotect++;
SEXP in = PROTECT(chin(key, ans_names)); nprotect++;
Expand Down

0 comments on commit 18ef60e

Please sign in to comment.