Front page | perl.perl5.porters |
Postings from May 2013
On Tue, May 28, 2013 at 04:28:50PM +0100, Nicholas Clark wrote: > void > DESTROY(self) > SV *self > PREINIT: > stcxt_t *cxt = (stcxt_t *)SvPVX(SvRV(self)); > PPCODE: > 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 ClarkThread Previous | Thread Next