search

ropensci / rsvg

SVG renderer for R based on librsvg2
Other
95 stars 1 forks source link

erring on the side of caution with more PROTECT/UNPROTECT #33

Closed coolbutuseless closed 2 years ago

coolbutuseless commented 2 years ago

R extension guide recommends erring on the side of caution with PROTECT/UNPROTECT - so I've ensured PROTECT() used for all allocVector(), and delayed UNPROTECT() until just prior to return().

This may be overboard. Feel free to ignore if I'm being too conservative here.

jeroen commented 2 years ago

Yeah this is really not needed. For example in the case below, you're not calling any R API's in between your allocation and unprotect, so there really no chance of an accidental gc here.

  SEXP res = PROTECT(Rf_allocVector(RAWSXP, buf.size));
  memcpy(RAW(res), buf.buf, buf.size);
  free(buf.buf);
  UNPROTECT(1);

Also if this was a problem, the cran rchck tool would have raised a warning about it....

coolbutuseless commented 2 years ago

Ah. OK. I thought the RAW() macro would be something to PROTECT() against - it's one of the examples on the R extensions page and the emphasis on "assume any macro can allocate".

Happy to leave it as your call.

jeroen commented 2 years ago

RAW in this case just gives us the pointer to the underlying buffer that we just allocated. So it's really redundant. But thanks for the suggestion :)