Closed p5pRT closed 6 years ago
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
Nicholas Clark wrote:
I think that we should abolish these two variables\, and replace them with local auto variables in functions that use them.
Agreed.
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
The RT System itself - Status changed from 'new' to 'open'
On Sat\, Mar 01\, 2014 at 02:46:47PM +0000\, Zefram wrote:
Nicholas Clark wrote:
I think that we should abolish these two variables\, and replace them with local auto variables in functions that use them.
Agreed.
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
Can PL_statcache also be removed?
-- bulk88 ~ bulk88 at hotmail.com
On Sat\, Mar 15\, 2014 at 11:56:01PM -0700\, bulk88 via RT wrote:
Can PL_statcache also be removed?
No\, it's used to save the result of the last stat so that -s _ etc work.
Tony
On Sat Mar 01 08:09:26 2014\, nicholas wrote:
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
On Sun Mar 22 20:15:53 2015\, bulk88 wrote:
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
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
On Tue Aug 11 11:20:13 2015\, bulk88 wrote:
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.
Another patch removing most of the usage of PL_statbuf. Since the PL_statbuf code is complicated and using global state\, it is best to remove it slowly to identify breakage/bisecting.
-- bulk88 ~ bulk88 at hotmail.com
On Fri Aug 14 21:14:28 2015\, bulk88 wrote:
On Tue Aug 11 11:20:13 2015\, bulk88 wrote:
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.
Another patch removing most of the usage of PL_statbuf. Since the PL_statbuf code is complicated and using global state\, it is best to remove it slowly to identify breakage/bisecting.
Bump.
-- bulk88 ~ bulk88 at hotmail.com
On Fri\, Sep 25\, 2015 at 03:08:49PM -0700\, bulk88 via RT wrote:
On Fri Aug 14 21:14:28 2015\, bulk88 wrote:
On Tue Aug 11 11:20:13 2015\, bulk88 wrote:
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.
Another patch removing most of the usage of PL_statbuf. Since the PL_statbuf code is complicated and using global state\, it is best to remove it slowly to identify breakage/bisecting.
Bump.
Thanks\, applied as
45a23732c73c80f49ab3dcbcbfc7ccc88aa98065
-- Music lesson: a symbiotic relationship whereby a pupil's embellishments concerning the amount of practice performed since the last lesson are rewarded with embellishments from the teacher concerning the pupil's progress over the corresponding period.
@iabyn - Status changed from 'open' to 'resolved'
On Thu Oct 08 08:57:29 2015\, davem wrote:
On Fri\, Sep 25\, 2015 at 03:08:49PM -0700\, bulk88 via RT wrote:
On Fri Aug 14 21:14:28 2015\, bulk88 wrote:
On Tue Aug 11 11:20:13 2015\, bulk88 wrote:
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.
Another patch removing most of the usage of PL_statbuf. Since the PL_statbuf code is complicated and using global state\, it is best to remove it slowly to identify breakage/bisecting.
Bump.
Thanks\, applied as
45a23732c73c80f49ab3dcbcbfc7ccc88aa98065
This ticket needs to be reopened\, it is not resolved. Patch "remove deprecated PL_timesbuf" was never applied. And this ticket needs to stays open until the original purpose of it "Replace use of PL_statbuf and PL_timesbuf with auto variables" is satisfied. Patch "partial PL_statbuf removal" didn't remove entirely remove PL_statbuf. Removing the last piece of PL_statbuf will probably be part of a refactor of "S_openn_cleanup" and "openn_setup" since those 2 take half a dozen or more "&some_c_auto"\, which sounds like a OPEN_STATE struct needs to created for doio.c and just one "OPEN_STATE *" passed to openn_cleanup/openn_setup instead of each member of a future OPEN_STATE being passed by reference one by one as args.
-- bulk88 ~ bulk88 at hotmail.com
@tonycoz - Status changed from 'resolved' to 'open'
bulk88 via RT \perlbug\-followup@​perl\.org wrote:
This ticket needs to be reopened\, it is not resolved. Patch "remove deprecated PL_timesbuf" was never applied.
Thanks\, applied (with tweaks to the perldelta entry\, and with a couple of additional but unimportant mentions of the variable removed) as c52cb8175c7c08890821789b4c7177b1e0e92558.
My understanding is that there are other parts of this ticket that are still outstanding\, so I am not resolving it.
-- Aaron Crane ** http://aaroncrane.co.uk/
Aaron Crane \arc@​cpan\.org writes:
bulk88 via RT \perlbug\-followup@​perl\.org wrote:
This ticket needs to be reopened\, it is not resolved. Patch "remove deprecated PL_timesbuf" was never applied.
Thanks\, applied (with tweaks to the perldelta entry\, and with a couple of additional but unimportant mentions of the variable removed) as c52cb8175c7c08890821789b4c7177b1e0e92558.
My understanding is that there are other parts of this ticket that are still outstanding\, so I am not resolving it.
Yes\, thre's some uses left in os2/os2.c and ext/ODBM_File/ODBM_File.xs\, plus a bunch of interwoven uses in doio.c (see https://rt.perl.org/Public/Bug/Display.html?id=121351#txn-1369717).
I'll push the following patch to remove the two former\, if nobody objects.
-- "A disappointingly low fraction of the human race is\, at any given time\, on fire." - Stig Sandbeck Mathisen
ilmari@ilmari.org (Dagfinn Ilmari Mannsåker) writes:
Yes\, thre's some uses left in os2/os2.c and ext/ODBM_File/ODBM_File.xs\, plus a bunch of interwoven uses in doio.c (see https://rt.perl.org/Public/Bug/Display.html?id=121351#txn-1369717).
I'll push the following patch to remove the two former\, if nobody objects.
Pushed as d50541e1f2ca48a36decdc63a6f00b99befb153f with the minor tweak of using Stat_t instead of struct stat in ODBM_File (but not os2\, since the rest of the os2 code uses struct stat anyway).
-- - Twitter seems more influential [than blogs] in the 'gets reported in the mainstream press' sense at least. - Matt McLeod - That'd be because the content of a tweet is easier to condense down to a mainstream media article. - Calle Dybedahl
PL_statbuf was finally removed in 5.27.1\, in commit 7e30e49f7461aeda1a5ab4539abfbe54f0f50e67. This ticket has been completed and can now be closed.
-zefram
@cpansprout - Status changed from 'open' to 'pending release'
Thank you for filing this report. You have helped make Perl better.
With the release yesterday of Perl 5.28.0\, this and 185 other issues have been resolved.
Perl 5.28.0 may be downloaded via: https://metacpan.org/release/XSAWYERX/perl-5.28.0
If you find that the problem persists\, feel free to reopen this ticket.
@khwilliamson - Status changed from 'pending release' to 'resolved'
Migrated from rt.perl.org#121351 (status was 'resolved')
Searchable as RT121351$