[R-pkg-devel] Notes from rchk on hdf5r package

Using this line of code as an example:

    SEXP hdf5r_ns = PROTECT(eval(lang2(install("getNamespace"),
mkString("hdf5r")), R_GlobalEnv));

The issue is fairly nuanced. There are a few main things to note
before we can understand the issue:

    1. The order of evaluation of arguments in C is unspecified, and
so arguments might be evaluated in any order depending on what the
compiler decides to do,
    2. `mkString()`, like other R APIs, creates an unprotected vector,
    3. `install()` can evaluate, if the symbol "getNamespace" has not
yet been interned into R's symbol table.

That implies that the following could potentially happen:

    1. First, an (unprotected!) R object is created by 'mkString("hdf5r")',
    2. Next, the invocation of `install("getNamespace")` causes an
allocation in R,
    3. That allocation triggers the garbage collector,
    4. The string created in 1. is erroneously collected.

While normally the SEXP created by `mkString()` would implicitly be
protected because it lives within the language object created by
`lang2()`, this doesn't happen in time since `install()` is evaluated
before `lang2()`.

So your intuition is correct: you should protect the result of
`mkString()` before putting it into the list.

For the second case:

    eval(lang3(install("set_ref.H5R"), result, _Robj), hdf5r_ns);

`eval()` does not protect its arguments, and `lang3()` creates an
unprotected SEXP. This implies it could be garbage collected during
execution. This would imply that calls like `sys.calls()`, which
attempt to inspect the set of calls, could break (as the SEXP used for
evaluation of that call has since been cleaned up).

In other words, you should protect language objects before evaluating them.

Best,
Kevin