Re: [perl #127697] newPVOP documentation is misleading

On Mon, Mar 14, 2016 at 09:33:54AM +0000, Dave Mitchell wrote:
> On Sat, Mar 12, 2016 at 03:04:16PM -0800, l.mai@web.de wrote:
> > # New Ticket Created by  l.mai@web.de 
> > # Please include the string:  [perl #127697]
> > # in the subject line of all future correspondence about this issue. 
> > # <URL: https://rt.perl.org/Ticket/Display.html?id=127697 >
> > 
> > 
> > 
> > This is a bug report for perl from l.mai@web.de,
> > generated with the help of perlbug 1.40 running under perl 5.22.1.
> > 
> > 
> > -----------------------------------------------------------------
> > [Please describe your issue here]
> > 
> > perldoc perlapi:
> > 
> >     newPVOP Constructs, checks, and returns an op of any type that involves an
> >             embedded C-level pointer (PV). type is the opcode. flags gives the
> >             eight bits of "op_flags". pv supplies the C-level pointer, which
> >             must have been allocated using "PerlMemShared_malloc"; the memory
> >             will be freed when the op is destroyed.
> > 
> >                     OP *    newPVOP(I32 type, I32 flags, char *pv)
> > 
> > 
> > "the memory will be freed when the op is destroyed" is arguably wrong. The
> > default action of perl is to do nothing and leak the memory. pv is only freed
> > if type is one of OP_DUMP, OP_GOTO, OP_NEXT, OP_LAST, OP_REDO, OP_TRANS,
> > OP_TRANSR, and op_flags and op_private have the right values.
> 
> I suppose the question is whether to fix the code or the docs.
> I could see op_clear() doing a PerlMemShared_free(cPVOPo->op_pv)
> if the op type is OP_CUSTOM and xop_class is OA_PVOP_OR_SVOP.

I still don't know the answer to that.

> Having had a quick look at op_clear(), I also note that there is a latent
> bug in op_pv field handling: the OP_DUMP etc cases fall through to the
> OP_TRANS cases, which check for OPpTRANS_FROM_UTF etc private flags. If
> those flags should at some point in the future overlap with private flags
> in OP_DUMP etc, then things will go wrong.

I've fixed that now with v5.25.9-32-gabd07ec

-- 
This is a great day for France!
    -- Nixon at Charles De Gaulle's funeral

• [perl #127697] newPVOP documentation is misleading by l . mai @ web . de
• Re: [perl #127697] newPVOP documentation is misleading by Dave Mitchell
• Re: [perl #127697] newPVOP documentation is misleading by Dave Mitchell