On Sat, Apr 28, 2012 at 05:59:57PM -0700, Father Chrysostomos via RT wrote:
> On Sat Apr 28 17:41:43 2012, jkeenan wrote:
> > Now it could be that my implementation of these functions is simply
> > incorrect. Here they are:
> >
> > sub hashref_locked {
> > my $hash=shift;
> > Internals::SvREADONLY($hash) ? return 0 : return 1;
> > }
> >
> > sub hash_locked(\%) { hashref_locked(@_) }
> >
> > sub hashref_unlocked {
> > my $hash=shift;
> > (! Internals::SvREADONLY($hash)) ? return 1 : return 0;
> > }
>
> I think I responded a little too hastily earlier.
>
> You are returning the same thing in both hashref_locked and
> hashref_unlocked.
>
> In the first, you have ‘return 0’ when SvREADONLY is true.
>
> In the second, you have ‘return 0’ when !SvREADONLY is false.
>
> Your double negatives make my head hurt. I find it much easier to read
> like this (with hashref_locked corrected):
>
> sub hashref_locked {
> my $hash=shift;
> !Internals::SvREADONLY($hash);
> }
>
> sub hashref_unlocked {
> my $hash=shift;
> Internals::SvREADONLY($hash);
> }
>
> which makes the difference obvious.
>
> >
> > sub hash_unlocked(\%) { hashref_unlocked(@_) }
> >
> > As noted above, only the *_unlocked functions are implemented in
> > version
> > 0.11:
> >
> > sub hashref_unlocked {
> > my $hash=shift;
> > return Internals::SvREADONLY($hash)
> > }
>
> That’s wrong, because the SvREADONLY flag means locked.
>
> >
> > I don't really know what Internals::SvREADONLY is or
> > does.
>
> SvREADONLY will return true for any read-only scalar, like $].
>
> SvREADONLY on a hash means it is locked.
>
> Internally, at the C level, SvREADONLY also means copy-on-write, which
> is why the code you cite below uses SvIsCOW to distinguish the cases
> (i.e., Internals::SvREADONLY doesn’t strictly flip the flag on and off,
> as that is not what Hash::Util needs). sv_force_normal stops a COW
> scalar from being such.
>
> Knowing that, I find the code self-documenting, but I’m getting too
> close to the core these days to know what would be obvious to other
> people. :-)
Which makes me wonder why SvREADONLY is exposed to perl-space at all.
It's not like it even keeps Hash::Util from needing XS. Wouldn't it make
things a lot easier to follow if Internals::SvREADONLY was removed in
favor of just adding a few bits to ext/Hash-Util/Util.xs?
-doy
Thread Previous
|
Thread Next