Hi -
On Tue, Apr 15, 2014 at 03:40:39AM -0400, Nathan Scott wrote:
> [...]
> > Fair enough, but I'm reluctant to keep working on [666] more without a
> > definite "good enough" target.
>
> As mentioned on IRC, splitting the test into smaller parts may be one
> strategy. [...]
I see, so the problem is that the single test case takes 6 minutes,
but you'd be fine it if broken into three test cases they took 2
minutes each?
> [...]
> To avoid this, the list could be conditionally appended if PM_ERR_NAME
> is the error. Could you try that out with your test case? [...]
> Beware though - it could be a really long list of names - are we sure
> we want to do this? (may result in potentially long list of names that
> we dump on stderr, typically). Not sure, not sure at all.
There is no requirement that the pmErr exception object produce a
super-long textual report. The __str__ method could abbreviate.
> > > Wow, amazed that I have not ever observed this! Do any/all of
> > > tests 739, 741, 742, 743, 991 fail for you? [...]
> >
> > No.
>
> OK - so we have no tests that demonstrate the problem, on any python
> version / test platforms? That should be addressed.
Sure, but that's simply because those test cases don't print the usage
text, so they don't notice any corruption of the
mistakenly-assumed-stable pmProgname char*.
> > > Does the patch attached resolve the problem you observed?
> >
> > I'm not sure it's sufficient, and IMHO the patch is worse
>
> So ... you didn't try it out for me on your test case? (please do &
> let me know, and could you add the python test case to the testsuite
> please? this discussion is made more difficult because we're talking
> about abstract instead of concrete test cases).
Sorry. The problem seemed perfectly concrete, small, and was
incidental to the development going on elsewhere so I didn't bother
create a stripped-down test case at the time.
> libpcp has taken this pragmatic approach for so many years - I think
> its just fine. There are no performance tools that I've come across
> that would ever want to change their argv[0] after starting. So, I'm
> going for "practicality beats purity" and limiting the scope of the
> change, particularly as we are so close to a release.
FWIW, I see nothing "impractical" about strdup, which your own patch
does in another context, but opinions vary so. Anyway, if we are to
stay with this assumption, then it should be documented so if
anybody's future PCP application uses PMAPI and decides to change
their argv[] and get corrupted error and usage messages or logfile
names, we can throw the book at them.
> [...] So in the end, I've taken another look though and have
> attempted to produce a best-of-both-worlds approach for this
> release.
(IMHO, this much discussion & revision is out of proportion with one
small working patch.)
- FChE
|