develooper Front page | perl.perl5.porters | Postings from February 2018

Re: [perl #131046] [PATCH] Carp: Do not crash when reading @DB::args

Thread Previous | Thread Next
February 24, 2018 00:00
Re: [perl #131046] [PATCH] Carp: Do not crash when reading @DB::args
Message ID:
demerphq wrote:
>Patch pushed as 4764858cb80e76fdba33cc1b3be8fcdef26df754

There are a couple of problems with this patch, going beyond my
distaste for the inadequacy of the code change.  Firstly, the commit
message contains exactly the kind of misconception that I argued against.
Rather than describing the effect as partial, the message says "This patch
safely iterates all elements", "This prevent crashing perl", and "It [is]
... workaround for Carp module".  (Bad grammar all in the actual message.)
This is describing the patch as having an effectiveness that it doesn't.

Secondly, the newly added test is predicated on that very same mythical
effectiveness.  It sets up a stack refcounting failure, at which point
behaviour is undefined, whether the Carp patch is applied or not.
This is not a correct test: it can fail without anything getting more
wrong than it already is.

The last argument advanced in favour of this patch, unlike many that
had preceded it, was supposedly based on a correct understanding of its
limitations.  I didn't argue against that, because my greater objection
is about the proposed patch being misunderstood and oversold.  Where it's
correctly understood, it's more a matter of opinion whether it's useful.
But the patch as applied doesn't seem to have taken that correct
understanding into account.  It is firmly based on the misconception,
and I seem to have been fooled into dropping my objections by some talk
that doesn't actually reflect the totality of the patch.

Bonus problem: the patch desynchronises the version numbers of Carp and
Carp::Heavy, giving the former an unjustified underscored version.

Clearly the new test needs to be deleted.  I don't know what to do about
the false commit message, though.  The message doesn't reference this
ticket, so putting a correction here doesn't serve as an annotation.
I'm tempted to revert the commit, on the basis that the code change can
later be applied again with a correct message.  Then at least git blame
would point at something correct.


Thread Previous | Thread Next Perl Programming lists via nntp and http.
Comments to Ask Bjørn Hansen at | Group listing | About