From 771218a62a0066949f51c0df68ea3ff58717ba27 Mon Sep 17 00:00:00 2001 From: Joshua Ulrich Date: Sun, 30 Aug 2020 07:01:00 -0500 Subject: [PATCH] Move checks to zlema() C code, fix 'n' calculation This uses many of the same checks, in the same order, as the ema() C function. That makes it easier and safer to call zlema() from other C functions. This also fixes the issue flagged by clang-UBSAN: We approximated 'n' using 'ratio', but it was possible that ratio = 0. There was a check for ratio > 0 later in the function, but that didn't prevent the possibility of division by 0. Thanks to Prof Ripley for the report. See #100, see #69. --- src/moving_averages.c | 36 ++++++++++++++++++++---------------- 1 file changed, 20 insertions(+), 16 deletions(-) diff --git a/src/moving_averages.c b/src/moving_averages.c index 9468444..1c8ea19 100644 --- a/src/moving_averages.c +++ b/src/moving_averages.c @@ -286,25 +286,29 @@ SEXP zlema (SEXP x, SEXP n, SEXP ratio) { } double *d_x = REAL(x); - /* If ratio is specified, and n is not, then - * set n to approx 'correct' value backed out from ratio - */ - int i_n; - double d_ratio; - if(R_NilValue == n && R_NilValue != ratio) { - d_ratio = asReal(ratio); - i_n = (int)(2.0 / d_ratio - 1.0); - } else { - i_n = asInteger(n); + if(ncols(x) > 1) { + error("ncol(x) > 1; ZLEMA only supports univariate 'x'"); } - /* Determine decay ratio */ - if(R_NilValue == ratio) { - d_ratio = 2.0 / (i_n + 1); + int i_n = asInteger(n); + double d_ratio = asReal(ratio); + + if(R_NilValue == n || i_n <= 0) { + if(R_NilValue == ratio || d_ratio <= 0.0) { + error("either 'n' or 'ratio' must be specified and > 0\n", + "'n' is ", n, " 'ratio' is ", ratio); + } else { + /* If ratio is specified, and n is not, set n to approx 'correct' + * value backed out from ratio */ + i_n = (int)(2.0 / d_ratio - 1.0); + } } else { - d_ratio = asReal(ratio); - if(d_ratio <= 0.0) { - error("'ratio' must be > 0"); + /* Determine decay ratio */ + if(R_NilValue == ratio) { + d_ratio = 2.0 / (i_n + 1); + } else { + /* ratio != NULL -> warn that 'n' will be used instead */ + warning("both 'n' and 'ratio' are specified; using 'n'"); } }