Discussion:
[perl #121351] Replace use of PL_statbuf and PL_timesbuf with auto variables
Nicholas Clark (via RT)
2014-02-28 16:19:10 UTC
Permalink
# New Ticket Created by Nicholas Clark
# Please include the string: [perl #121351]
# in the subject line of all future correspondence about this issue.
# <URL: https://rt.perl.org/Ticket/Display.html?id=121351 >


intrpvar.h contains these two lines:

PERLVAR(I, statbuf, Stat_t)

and

PERLVAR(I, timesbuf, struct tms)


(Note that the stat struct used to implement _ is in PL_statcache,
which is not the same as PL_statbuf)

These variables are not used to pass any state between functions in
the perl core, and aren't (meaningfully) used by any modules on CPAN:

http://grep.cpan.me/?q=PL_statbuf
http://grep.cpan.me/?q=PL_timesbuf

(note that PAR-Packer must already work just fine without PL_statbuf,
as PL_statbuf is *not* a macro on an unthreaded perl, and so its
#ifndef PL_statbuf code will already be used)

These seem to be vestiges of Perl 1, which has this in its perl.h:

EXT struct stat statbuf;
EXT struct tms timesbuf;

I asked Larry if he remembered why:

16:05 < nwc10> TimToady: in perl 1, why does it have EXT struct stat statbuf; and EXT struct tms timesbuf; in perl.h, rather than using local variables in functions? Was there some compiler back then that screwed up allocating structs on the stack?
16:09 < TimToady> Don't recall, but I suspect it's just because the stat manpage had 'extern struct stat statbuf'
16:09 < nwc10> aha thanks

http://irclog.perlgeek.de/perl6/2014-02-28#i_8363520


I think that we should abolish these two variables, and replace them
with local auto variables in functions that use them.

I think that we shouldn't do this before v5.20.0 escapes.

Given the complete lack of CPAN usage, I'm also not sure whether we
should deprecate them, or just remove them. I think that we don't
actually yet have a way to add C compiler annotations in intrpvar.h,
but we probably could fix that if needed.

Nicholas Clark
Zefram
2014-03-01 14:46:47 UTC
Permalink
Post by Nicholas Clark (via RT)
I think that we should abolish these two variables, and replace them
with local auto variables in functions that use them.
Agreed.
Post by Nicholas Clark (via RT)
Given the complete lack of CPAN usage, I'm also not sure whether we
should deprecate them, or just remove them.
Just remove them, early in 5.21. Even if they are used, a year is plenty
of time for XS authors to make such a trivial change.

-zefram
Nicholas Clark
2014-03-01 16:09:00 UTC
Permalink
Post by Zefram
Post by Nicholas Clark (via RT)
I think that we should abolish these two variables, and replace them
with local auto variables in functions that use them.
Agreed.
Post by Nicholas Clark (via RT)
Given the complete lack of CPAN usage, I'm also not sure whether we
should deprecate them, or just remove them.
Just remove them, early in 5.21. Even if they are used, a year is plenty
of time for XS authors to make such a trivial change.
Thanks. I'd reached this conclusion too. I don't think that there's much
gain in creating a deprecation warning for something that when changed
breaks as an obvious compile-time hard failure. In that, unlike some
Perl-space changes, in this case no installed running systems can be broken
by the C header change, because you simply can't install what you can't
build.

Nicholas Clark
bulk88 via RT
2015-03-23 03:15:54 UTC
Permalink
Post by Nicholas Clark
Thanks. I'd reached this conclusion too. I don't think that there's much
gain in creating a deprecation warning for something that when changed
breaks as an obvious compile-time hard failure. In that, unlike some
Perl-space changes, in this case no installed running systems can be broken
by the C header change, because you simply can't install what you can't
build.
Nicholas Clark
This ticket got a commit from NC at http://perl5.git.perl.org/perl.git/commit/25983af42cdcf2dc1fea6717dac7aac441b6301d I think this ticket can be closed.
--
bulk88 ~ bulk88 at hotmail.com

---
via perlbug: queue: perl5 status: open
https://rt.perl.org/Ticket/Display.html?id=121351
bulk88 via RT
2015-03-23 03:19:04 UTC
Permalink
Post by bulk88 via RT
This ticket got a commit from NC at
http://perl5.git.perl.org/perl.git/commit/25983af42cdcf2dc1fea6717dac7aac441b6301d
I think this ticket can be closed.
Actually its a 5.23 blocker now, that commit didn't remove the interp var http://perl5.git.perl.org/perl.git/blobdiff/c79d007613fa174f6f5e1588ca5374f505fc44af..25983af42cdcf2dc1fea6717dac7aac441b6301d:/intrpvar.h so that has to be done, it should have been done after blead was thawed after 5.20 was released, but NC never got around to it.
--
bulk88 ~ bulk88 at hotmail.com

---
via perlbug: queue: perl5 status: open
https://rt.perl.org/Ticket/Display.html?id=121351
bulk88 via RT
2015-08-11 18:20:13 UTC
Permalink
Patch attached. This ticket needs to still stay open since PL_statbuf hasn't been removed, it isn't since simple since I found an uninit memory problem with PL_statbuf that will take a while to solve.
--
bulk88 ~ bulk88 at hotmail.com

---
via perlbug: queue: perl5 status: open
https://rt.perl.org/Ticket/Display.html?id=121351
bulk88 via RT
2014-03-16 06:56:01 UTC
Permalink
Can PL_statcache also be removed?
--
bulk88 ~ bulk88 at hotmail.com

---
via perlbug: queue: perl5 status: open
https://rt.perl.org/Ticket/Display.html?id=121351
Tony Cook
2014-03-16 08:55:12 UTC
Permalink
Post by bulk88 via RT
Can PL_statcache also be removed?
No, it's used to save the result of the last stat so that -s _ etc
work.

Tony
Loading...