Skip to content

Commit

Permalink
Prevent writing out of bounds when final column in entirely NA
Browse files Browse the repository at this point in the history
If a column is entirely NA, then firstNonNACol will return the index
of the first entry of the next column.  In the case where the final
column is entirely NA then this will result in writing off the end of
the allocated result.

Fix by subtracting one from the index in this case.

Hopefully fixes #234
  • Loading branch information
TomAndrews committed Apr 18, 2018
1 parent 31b5911 commit f034f2b
Show file tree
Hide file tree
Showing 2 changed files with 22 additions and 0 deletions.
14 changes: 14 additions & 0 deletions inst/unitTests/runit.na.locf.R
Expand Up @@ -188,3 +188,17 @@ test.nalocf_by_column_1NA_fromLast <- function() {
checkEquals(x, as.xts(z), check.attributes = TRUE)
}
}

test.nalocf_last_column_all_NA <- function() {
nacol <- NCOL(XDAT2)
for (m in MODES) {
xdat <- XDAT2
xdat[,nacol] <- xdat[,nacol] * NA
storage.mode(xdat) <- m
zdat <- as.zoo(xdat)

x <- na.locf(xdat)
z <- na.locf(zdat)
checkEquals(x, as.xts(z), check.attributes = TRUE)
}
}
8 changes: 8 additions & 0 deletions src/na.c
Expand Up @@ -171,6 +171,8 @@ SEXP na_locf (SEXP x, SEXP fromLast, SEXP _maxgap, SEXP _limit)
for(j=0; j < nc; j++) {
/* copy leading NAs */
_first = firstNonNACol(x, j);
if (_first == nr + j*nr)
_first--;
for(i=0+j*nr; i < (_first+1); i++) {
int_result[i] = int_x[i];
}
Expand Down Expand Up @@ -211,6 +213,8 @@ SEXP na_locf (SEXP x, SEXP fromLast, SEXP _maxgap, SEXP _limit)
for(j=0; j < nc; j++) {
/* copy leading NAs */
_first = firstNonNACol(x, j);
if (_first == nr + j*nr)
_first--;
for(i=0+j*nr; i < (_first+1); i++) {
int_result[i] = int_x[i];
}
Expand Down Expand Up @@ -272,6 +276,8 @@ SEXP na_locf (SEXP x, SEXP fromLast, SEXP _maxgap, SEXP _limit)
for(j=0; j < nc; j++) {
/* copy leading NAs */
_first = firstNonNACol(x, j);
if (_first == nr + j*nr)
_first--;
for(i=0+j*nr; i < (_first+1); i++) {
real_result[i] = real_x[i];
}
Expand Down Expand Up @@ -330,6 +336,8 @@ SEXP na_locf (SEXP x, SEXP fromLast, SEXP _maxgap, SEXP _limit)
for(j=0; j < nc; j++) {
/* copy leading NAs */
_first = firstNonNACol(x, j);
if (_first == nr + j*nr)
_first--;
for(i=0+j*nr; i < (_first+1); i++) {
SET_STRING_ELT(result, i, STRING_ELT(x, i));
}
Expand Down

0 comments on commit f034f2b

Please sign in to comment.