On Mon, Feb 27, 2012 at 7:43 PM, David Malcolm <dmalcolm(a)redhat.com> wrote:
This adds:
__attribute__((cpychecker_negative_result_sets_exception))
Great, thank you for the quick roundtrip!
#define CPYCHECKER_NEGATIVE_RESULT_SETS_EXCEPTION() \
Note that you are proposing to use a macro with arguments: it looks
redundant, or at least not consistent with the other attribute macros.
This idiom is in the docs example too, as I can read from the patch.
Does this give you want you need?
It is mostly working. The problem I'm finding now is when the
exception is set in another function, for instance, the typical
implementation of these -1-raising functions is something like:
int something() {
rv = -1
/* work work ... */
if (rv < 0) {
pq_complete_error(self, &pgres, &error);
}
return rv;
}
these functions get a "function is marked with
__attribute__((cpychecker_negative_result_sets_exception)) but can
return -1 without setting an exception". I have re-defined
pq_complete_error from returning void to returning int, and always
returning -1, but this doesn't help. OTOH, if I say:
if (rv < 0) {
rv = pq_complete_error(self, &pgres, &error);
}
no warning is generated. Unfortunately this pattern is not optimal, as
rv could be a different negative return value, to specify different
failure conditions, and this detail would be swallowed.
Likely the same shortcoming, a function:
PyObject *f() {
rv = NULL;
if (bad_stuff) {
psyco_set_error(ProgrammingError, self, "reason", NULL, NULL);
goto exit;
}
/* work work */
rv = some_object();
exit:
return rv;
}
this function is flagged as "returning (PyObject*)NULL without setting
an exception" even if psyco_set_error is flagged as setting an
exception on negative return, and constantly returns -1. I have tried
changing the psyco_set_error() signature as returning a PyObject* and
always returning NULL, but this is even worse as then it looks I get a
value I don't decref.
Do you think it is something that can be fixed or I'm starting asking
too much to the checker? Would it be handy a self-containing example
triggering the false positive to be added to the test suite?
A separate, smallish issue, yet requiring "worsening the code" in
order to silence a false positive is a function returning a specific
object - XidObject - a "subclass" of PyObject.
A caller to the function "XidObject *xid_ensure(PyObject *o)":
XidObject *xid = NULL;
PyObject *oxid; /* from the args */
if (NULL == (xid = xid_ensure(oxid))) {
goto exit;
}
is also flagged as returning NULL without setting an exception. The
above statement specifically is marked as "when treating unknown
struct XidObject * from ... as NULL
taking True path".
Fixing the warning by changing the xid_ensure signature as returning
PyObject *, and using it as "xid = (XidObject *)xid_ensure(oxid)"
works, but is not ideal.
Is there any way to propagate the PyObject semantics to its subtypes?
Thank you very much for your active feedback.
-- Daniele