develooper Front page | perl.perl5.porters | Postings from May 2013

Re: [perl #118139] Storable in DESTROY blocks

Thread Previous | Thread Next
Nicholas Clark
May 28, 2013 16:17
Re: [perl #118139] Storable in DESTROY blocks
Message ID:
On Tue, May 28, 2013 at 04:28:50PM +0100, Nicholas Clark wrote:

> void
> DESTROY(self)
>     SV *self
> 	stcxt_t *cxt = (stcxt_t *)SvPVX(SvRV(self));
> 	if (kbuf)
> 		Safefree(kbuf);
> 	if (!cxt->membuf_ro && mbase)
> 		Safefree(mbase);
> 	if (cxt->membuf_ro && (cxt->msaved).arena)
> 		Safefree((cxt->msaved).arena);
> To de-obfuscate the above code you need to know this:
> #define kbuf	(cxt->keybuf).arena
> #define mbase	(cxt->membuf).arena

> I've not fully worked this out, but I think that it's only using bless to
> ensure that it gets to run that code to call free(). In turn, given that
> cxt->membuf_ro gets to select one of two places to have a pointer to
> (seemingly) the same thing, I'm wondering if that can be simplified. At
> which point there are only two dynamic sized buffers, and one fixed
> structure. Which seems equivalent to two dynamic sized buffers. Which might
> be possible to implement without needing malloc(), free(), bless and DESTROY.

Right. As best I can tell the dance between ->msaved and ->membuf is a bit
daft. The code looks like it can be refactored so that it instead has an
"out" buffer that is dynamically allocated and owned by the context, and an
"in" buffer that is just a pointer to the SV that is being read from.

Secondly, and more usefully, keybuf and membuf aren't actually needed at
the same time. keybuf is only used as scratch space to read in hash keys.
The code is also sufficiently abstracted between memory and file descriptor
reads that it "READ"s into keybuf from the input source, either way.
However, for the case of the input source being memory, the hash key is
already nicely lined up in memory. So by busting the abstraction a bit, and
peeking direct, one could completely avoid the need to have both keybuf and
membuf. Which means only one dynamically allocated storage pool per context.
Which gives several options to avoid needing that DESTROY.

I'm not sure how much sanity it will cost to refactor this.

Nicholas Clark

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