Author pitrou
Recipients Garen, belopolsky, benjamin.peterson, danchr, dhduvall, dmalcolm, fche, glyph, hazmat, jbaker, jcea, jmcp, laca, lasizoillo, loewis, mjw, movement, neologix, pitrou, rhettinger, robert.kern, ronaldoussoren, scox, serverhorror, sirg3, techtonik, twleung, wsanchez
Date 2012-01-21.23:16:15
SpamBayes Score 3.78586e-14
Marked as misclassified No
Message-id <1327187656.3382.40.camel@localhost.localdomain>
In-reply-to <1327104223.53.0.217811935108.issue13405@psf.upfronthosting.co.za>
Content
> So, yes. The code is intrusive. The code deals with a lot of internal
> machinery (PEP393 support in the ustack helper was quite difficult).
> It is going to break from time to time, sure. At the same time, I am
> committed to support it. And even if it is dropped in 3.4, no Python
> program will be affected.

To ease the concerns, I think you should make it so that dtrace-specific
code gets out of the way as much as possible.
I suggest you create a Python/ceval-dtrace.h header and put most
dtrace-specific code from ceval.c there. Its inclusion should be
conditional on WITH_DTRACE so that other core devs can ignore its
presence.

A couple other comments:
- in the makefile, DTRACEOBJS is inconsistent with DTRACE_STATIC and
LIBRARY_OBJS. You should make it DTRACE_OBJS.
- please add comments at the top of whatever header files you add, to
make it clear that they are dtrace-specific. Mentions of "ustack helper"
are a bit too specific to be helpful.
- some code lacks error checking, e.g. when calling PyUnicode_AsUTF8.
- is co_linenos ever freed or is it a memory leak?
- your indices and offsets should be Py_ssize_t, not int
- is an empty dtrace module really needed? a flag variable in the sys
module should be enough
- as you can see the Makefile uses "-rm -f", you should probably do the
same instead of "rm -f"
- you have a rather strange "if true" in your configure.in additions

Thanks in advance.
History
Date User Action Args
2012-01-21 23:16:17pitrousetrecipients: + pitrou, loewis, rhettinger, jcea, ronaldoussoren, belopolsky, wsanchez, movement, techtonik, benjamin.peterson, serverhorror, glyph, laca, twleung, jbaker, robert.kern, sirg3, danchr, dhduvall, dmalcolm, mjw, Garen, neologix, lasizoillo, fche, hazmat, jmcp, scox
2012-01-21 23:16:16pitroulinkissue13405 messages
2012-01-21 23:16:15pitroucreate