On 04/07/2017 05:37 AM, luke-tier...@uiowa.edu wrote:
On Fri, 7 Apr 2017, Hervé Pagès wrote:

On 04/06/2017 03:29 AM, Michael Lawrence wrote:
On Thu, Apr 6, 2017 at 2:59 AM, Martin Morgan
<martin.mor...@roswellpark.org> wrote:
On 04/06/2017 05:33 AM, Aaron Lun wrote:

The tool is not perfect, so assess each report carefully.

I get a lot of warnings because the tool seems to consider that
extracting an attribute (with getAttrib(x, ...)) or extracting the
slot of an S4 object (with GET_SLOT(x, ...) or R_do_slot(x, ...))
returns an SEXP that needs protection. I always assumed that it
didn't because my understanding is that the returned SEXP is pointing
to a part of a pre-existing object ('x') and not to a newly created
one. So I decided I could treat it like the SEXP returned by
VECTOR_ELT(), which, AFAIK, doesn't need protection.

So I suspect these warnings are false positives but I'm not 100% sure.

If you are not 100% sure then you should protect :-)

There are some cases, in particular related to compact row names on
data frames, where getAttrib will allocate.

Seriously? So setAttrib(x, ..., getAttrib) is not going to be a no-op
anymore? Should I worry that VECTOR_ELT() will also expand some sort
of compact list element? Why not keep these things low-level
getters/setters that return whatever the real thing is and use
higher-level accessors for returning the expanded version of the thing?

Thanks,
H.


Best,

luke




I also get a warning on almost every C++ function I've written,
because
I use the following code to handle exceptions:

     SEXP output=PROTECT(allocVector(...));
     try {
         // do something that might raise an exception
     } catch (std::exception& e) {
         UNPROTECT(1);
         throw; // break out of this part of the function
     }
     UNPROTECT(1);
     return output;

Presumably the check doesn't account for transfer of control to the
catch block. I find that R itself is pretty good at complaining about
stack imbalances during execution of tests, examples, etc.

'My' packages
(Rsamtools, DirichletMultinomial) had several false positives (all
associated with use of an attribute of a protected SEXP), one subtle
problem (a symbol from a PROTECT'ed package name space; the symbol
could
in theory be an active binding and the value obtained not
PROTECTed by
the name space), and a genuine bug

                tag = NEW_CHARACTER(n);
                for (int j = 0; j < n; ++j)
                    SET_STRING_ELT(tag, j, NA_STRING);
                if ('A' == aux[0]) {
                    buf_A = R_alloc(2, sizeof(char));  # <<- bug
                    buf_A[1] = '\0';
                }
                ...
                SET_VECTOR_ELT(tags, i, tag); # PROTECT tag, too
late!


I assume the bug refers to the un-PROTECT'd NEW_CHARACTER here - the
R_alloc call looks okay to me...


yes, tag needs protection.

I attributed the bug to R_alloc because I failed to reason that R_alloc
(obviously) allocates and hence can trigger a garbage collection.

Somehow it reflects my approach to PROTECTion, probably not shared by
everyone. I like to PROTECT only when necessary, rather than
indiscriminately. Probably this has no practical consequence in
terms of
performance, making the code a little easier to read at the expense of
exposing me to bugs like this.


I guess it's a tradeoff between syntactic complexity and logical
complexity. You have to think pretty hard to minimize use of the
protect stack.

I prefer to call it logical obscurity ;-)

The hard thinking consists in assessing whether or not the code between
the line where a new SEXP is allocated (line X) and the line where
it's put in a safe place (line Y) can trigger garbage collection.
Hard to figure out in general indeed, but not impossible! Problem
is that the result of this assessment is valid at a certain point
in time but might change in the future, even if your code has not
changed.

So a dangerous game for virtually zero benefits.


One recommendation might be to UNPROTECT() as soon as the pointer on
the top is unneeded, rather than trying to figure out the number to
pop just before returning to R.

If you PROTECT() in a loop, you definitely want to do that. Otherwise,
does it make a big difference?


One thing that got me is that the order in which C evaluates function
call arguments is undefined. I did a lot of R_setAttrib(x,
install("foo"), allocBar()), thinking that the symbol would be
automatically protected, and allocBar() would not need protection,
since it happened last. Unfortunately, it might be evaluated first.

I got hit by this too long time ago but with defineVar() instead of
R_setAttrib():

 
https://urldefense.proofpoint.com/v2/url?u=https-3A__stat.ethz.ch_pipermail_r-2Ddevel_2008-2DJanuary_048040.html&d=DwID-g&c=eRAMFD45gAfqt84VtBcfhQ&r=BK7q3XeAvimeWdGbWY_wJYbW0WYiZvSXAJJKaaPhzWA&m=FscW1HcPCwUqtMwKVFDfd1NyW0oHh0tJOPdFb3C1IWk&s=O3CcB-Z_OkVKaC1aV0aIc5SCDNqGQrkvGSmPf0TRAsw&e=

H.


Btw, I think my package RGtk2 got the record: 1952 errors. Luckily
almost all of them happened inside a few macros and autogenerated
code.

Martin


Cheers,

Aaron
_______________________________________________
Bioc-devel@r-project.org mailing list
https://urldefense.proofpoint.com/v2/url?u=https-3A__stat.ethz.ch_mailman_listinfo_bioc-2Ddevel&d=DwICAg&c=eRAMFD45gAfqt84VtBcfhQ&r=BK7q3XeAvimeWdGbWY_wJYbW0WYiZvSXAJJKaaPhzWA&m=su20YzStywMa_pdWblzF0RnK8ATRw-t61lOIOsi0xTU&s=JhBOw1ac5wXfV1BSjFuidxFiBTx43J7iEvZG4G0_0uU&e=




This email message may contain legally privileged
and/or...{{dropped:2}}

_______________________________________________
Bioc-devel@r-project.org mailing list
https://urldefense.proofpoint.com/v2/url?u=https-3A__stat.ethz.ch_mailman_listinfo_bioc-2Ddevel&d=DwICAg&c=eRAMFD45gAfqt84VtBcfhQ&r=BK7q3XeAvimeWdGbWY_wJYbW0WYiZvSXAJJKaaPhzWA&m=su20YzStywMa_pdWblzF0RnK8ATRw-t61lOIOsi0xTU&s=JhBOw1ac5wXfV1BSjFuidxFiBTx43J7iEvZG4G0_0uU&e=


_______________________________________________
Bioc-devel@r-project.org mailing list
https://urldefense.proofpoint.com/v2/url?u=https-3A__stat.ethz.ch_mailman_listinfo_bioc-2Ddevel&d=DwICAg&c=eRAMFD45gAfqt84VtBcfhQ&r=BK7q3XeAvimeWdGbWY_wJYbW0WYiZvSXAJJKaaPhzWA&m=su20YzStywMa_pdWblzF0RnK8ATRw-t61lOIOsi0xTU&s=JhBOw1ac5wXfV1BSjFuidxFiBTx43J7iEvZG4G0_0uU&e=






--
Hervé Pagès

Program in Computational Biology
Division of Public Health Sciences
Fred Hutchinson Cancer Research Center
1100 Fairview Ave. N, M1-B514
P.O. Box 19024
Seattle, WA 98109-1024

E-mail: hpa...@fredhutch.org
Phone:  (206) 667-5791
Fax:    (206) 667-1319

_______________________________________________
Bioc-devel@r-project.org mailing list
https://stat.ethz.ch/mailman/listinfo/bioc-devel

Reply via email to