Skip Menu |
Report information
Id: 121351
Status: resolved
Priority: 0/
Queue: perl5

Owner: Nobody
Requestors: nicholas <nick [at] ccl4.org>
Cc:
AdminCc:

Operating System: All
PatchStatus: Applied
Severity: low
Type:
Perl Version:
  • 1.0
  • 5.000
  • 5.002
  • 5.003
  • 5.004
  • 5.004_00
  • 5.004_01
  • 5.004_02
  • 5.004_03
  • 5.004_04
  • 5.004_05
  • 5.005
  • 5.005_01
  • 5.005_02
  • 5.005_03
  • 5.005_04
  • 5.6.0
  • 5.6.1
  • 5.6.2
  • 5.7.0
  • 5.7.1
  • 5.7.2
  • 5.8.0
  • 5.8.1
  • 5.8.2
  • 5.8.3
  • 5.8.4
  • 5.8.5
  • 5.8.6
  • 5.8.7
  • 5.8.8
  • 5.8.9
  • 5.9.0
  • 5.9.1
  • 5.9.2
  • 5.9.3
  • 5.9.4
  • 5.9.5
  • 5.10.0
  • 5.10.1
  • 5.11.0
  • 5.11.1
  • 5.11.2
  • 5.11.3
  • 5.11.4
  • 5.11.5
  • 5.12.0
  • 5.12.1
  • 5.12.2
  • 5.12.3
  • 5.12.4
  • 5.12.5
  • 5.12.6
  • 5.13.0
  • 5.13.1
  • 5.13.2
  • 5.13.3
  • 5.13.4
  • 5.13.5
  • 5.13.6
  • 5.13.7
  • 5.13.8
  • 5.13.9
  • 5.13.10
  • 5.13.11
  • 5.14.0
  • 5.14.1
  • 5.14.2
  • 5.14.3
  • 5.14.4
  • 5.14.5
  • 5.15.0
  • 5.15.1
  • 5.15.2
  • 5.15.3
  • 5.15.4
  • 5.15.5
  • 5.15.6
  • 5.15.7
  • 5.15.8
  • 5.15.9
  • 5.15.10
  • 5.16.0
  • 5.16.1
  • 5.16.2
  • 5.16.3
  • 5.16.4
  • 5.17.0
  • 5.17.1
  • 5.17.2
  • 5.17.3
  • 5.17.4
  • 5.17.5
  • 5.17.6
  • 5.17.7
  • 5.17.8
  • 5.17.9
  • 5.17.10
  • 5.17.11
  • 5.17.12
  • 5.18.0
  • 5.18.1
  • 5.19.0
  • 5.19.1
  • 5.19.2
  • 5.19.3
  • 5.19.4
  • 5.19.5
  • 5.19.6
  • 5.19.7
  • 5.19.8
  • 5.19.9
  • 5.19.10
Fixed In: (no value)

Attachments


Subject: Replace use of PL_statbuf and PL_timesbuf with auto variables
Download (untitled) / with headers
text/plain 1.6k
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
From: Zefram <zefram [...] fysh.org>
Date: Sat, 1 Mar 2014 14:46:47 +0000
To: perl5-porters [...] perl.org
Subject: Re: [perl #121351] Replace use of PL_statbuf and PL_timesbuf with auto variables
Download (untitled) / with headers
text/plain 410b
Nicholas Clark wrote: Show quoted text
>I think that we should abolish these two variables, and replace them >with local auto variables in functions that use them.
Agreed. Show quoted text
>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
Subject: Re: [perl #121351] Replace use of PL_statbuf and PL_timesbuf with auto variables
CC: perl5-porters [...] perl.org
To: Zefram <zefram [...] fysh.org>
Date: Sat, 1 Mar 2014 16:09:00 +0000
From: Nicholas Clark <nick [...] ccl4.org>
Download (untitled) / with headers
text/plain 864b
On Sat, Mar 01, 2014 at 02:46:47PM +0000, Zefram wrote: Show quoted text
> 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
RT-Send-CC: perl5-porters [...] perl.org
Can PL_statcache also be removed? -- bulk88 ~ bulk88 at hotmail.com
CC: perl5-porters [...] perl.org
Subject: Re: [perl #121351] Replace use of PL_statbuf and PL_timesbuf with auto variables
Date: Sun, 16 Mar 2014 19:55:12 +1100
To: bulk88 via RT <perlbug-followup [...] perl.org>
From: Tony Cook <tony [...] develop-help.com>
Download (untitled) / with headers
text/plain 180b
On Sat, Mar 15, 2014 at 11:56:01PM -0700, bulk88 via RT wrote: Show quoted text
> Can PL_statcache also be removed?
No, it's used to save the result of the last stat so that -s _ etc work. Tony
RT-Send-CC: perl5-porters [...] perl.org
Download (untitled) / with headers
text/plain 638b
On Sat Mar 01 08:09:26 2014, nicholas wrote: Show quoted text
> 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
RT-Send-CC: perl5-porters [...] perl.org
Download (untitled) / with headers
text/plain 580b
On Sun Mar 22 20:15:53 2015, bulk88 wrote: Show quoted text
> 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
Subject: [PATCH] Replace use of PL_statbuf and PL_timesbuf with auto variables
RT-Send-CC: perl5-porters [...] perl.org
Download (untitled) / with headers
text/plain 236b
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
Subject: 0001-121351-remove-deprecated-PL_timesbuf.patch
From 5856cdd25bb313c61eaddb62ce7cac58d5a22fc3 Mon Sep 17 00:00:00 2001 From: Daniel Dragan <bulk88@hotmail.com> Date: Tue, 11 Aug 2015 14:17:52 -0400 Subject: [PATCH] [#121351] remove deprecated PL_timesbuf Saves memory in interp struct. --- embedvar.h | 1 - intrpvar.h | 5 ----- pod/perldelta.pod | 3 ++- 3 files changed, 2 insertions(+), 7 deletions(-) diff --git a/embedvar.h b/embedvar.h index 9ed30e0..f6aaadf 100644 --- a/embedvar.h +++ b/embedvar.h @@ -323,7 +323,6 @@ #define PL_tainted (vTHX->Itainted) #define PL_tainting (vTHX->Itainting) #define PL_threadhook (vTHX->Ithreadhook) -#define PL_timesbuf (vTHX->Itimesbuf) #define PL_tmps_floor (vTHX->Itmps_floor) #define PL_tmps_ix (vTHX->Itmps_ix) #define PL_tmps_max (vTHX->Itmps_max) diff --git a/intrpvar.h b/intrpvar.h index 6ee88b3..337bf4f 100644 --- a/intrpvar.h +++ b/intrpvar.h @@ -178,11 +178,6 @@ PERLVAR(I, statcache, Stat_t) /* _ */ PERLVAR(I, statgv, GV *) PERLVARI(I, statname, SV *, NULL) -#ifdef HAS_TIMES -/* Will be removed soon after v5.23.2. See RT #121351 */ -PERLVAR(I, timesbuf, struct tms) -#endif - /* =for apidoc mn|SV*|PL_rs diff --git a/pod/perldelta.pod b/pod/perldelta.pod index 6ace170..38fd118 100644 --- a/pod/perldelta.pod +++ b/pod/perldelta.pod @@ -334,7 +334,8 @@ well. =item * -XXX +Deprecated in 5.20 interpreter variable C<PL_timesbuf> has been removed. +[perl #121351] =back -- 1.7.9.msysgit.0
RT-Send-CC: perl5-porters [...] perl.org
Download (untitled) / with headers
text/plain 470b
On Tue Aug 11 11:20:13 2015, bulk88 wrote: Show quoted text
> 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
Subject: 0001-121351-partial-PL_statbuf-removal.patch
From f3f2517215490c31e1646dfdcf6789f382d142e8 Mon Sep 17 00:00:00 2001 From: Daniel Dragan <bulk88@hotmail.com> Date: Fri, 14 Aug 2015 18:34:09 -0400 Subject: [PATCH] [#121351] partial PL_statbuf removal Perl_nextargv has to have access to the Stat_t that is written to inside S_openn_cleanup or else run/switches.t, io/argv.t, io/inplace.t, and io/iprefix.t will fail. Removing the uses of PL_statbuf that are using PL_statbuf due to historical reason, and not using PL_statbuf to pass data between different funcs/different callstacks. This patch makes it easier to remove PL_statbuf in the future since the number uses of it has been reduced. -in Perl_apply move SETERRNO before tot--; so the branch can be combined with other "tot--;" branches by CC optmizer -combine 2 Perl_croak(aTHX_ "Illegal suidscript"); statements in S_validate_suid to make code look simpler, drop my_perl arg for space efficiency on threads of rarely executed code --- doio.c | 16 +++++++++------- perl.c | 13 +++++-------- pp_hot.c | 3 ++- pp_sys.c | 10 +++++++--- util.c | 37 +++++++++++++++++++++++-------------- 5 files changed, 46 insertions(+), 33 deletions(-) diff --git a/doio.c b/doio.c index 39e5ce7..c196d74 100644 --- a/doio.c +++ b/doio.c @@ -835,6 +835,7 @@ Perl_nextargv(pTHX_ GV *gv, bool nomagicopen) if (!GvAV(gv)) return NULL; while (av_tindex(GvAV(gv)) >= 0) { + Stat_t statbuf; STRLEN oldlen; SV *const sv = av_shift(GvAV(gv)); SAVEFREESV(sv); @@ -976,13 +977,13 @@ Perl_nextargv(pTHX_ GV *gv, bool nomagicopen) setdefout(PL_argvoutgv); PL_lastfd = PerlIO_fileno(IoIFP(GvIOp(PL_argvoutgv))); if (PL_lastfd >= 0) { - (void)PerlLIO_fstat(PL_lastfd,&PL_statbuf); + (void)PerlLIO_fstat(PL_lastfd,&statbuf); #ifdef HAS_FCHMOD (void)fchmod(PL_lastfd,PL_filemode); #else (void)PerlLIO_chmod(PL_oldname,PL_filemode); #endif - if (fileuid != PL_statbuf.st_uid || filegid != PL_statbuf.st_gid) { + if (fileuid != statbuf.st_uid || filegid != statbuf.st_gid) { /* XXX silently ignore failures */ #ifdef HAS_FCHOWN PERL_UNUSED_RESULT(fchown(PL_lastfd,fileuid,filegid)); @@ -999,8 +1000,8 @@ Perl_nextargv(pTHX_ GV *gv, bool nomagicopen) if (ckWARN_d(WARN_INPLACE)) { const int eno = errno; - if (PerlLIO_stat(PL_oldname, &PL_statbuf) >= 0 - && !S_ISREG(PL_statbuf.st_mode)) { + if (PerlLIO_stat(PL_oldname, &statbuf) >= 0 + && !S_ISREG(statbuf.st_mode)) { Perl_warner(aTHX_ packWARN(WARN_INPLACE), "Can't do inplace edit: %s is not a regular file", PL_oldname); @@ -1955,11 +1956,12 @@ nothing in the core. tot--; } else { /* don't let root wipe out directories without -U */ - if (PerlLIO_lstat(s,&PL_statbuf) < 0) - tot--; - else if (S_ISDIR(PL_statbuf.st_mode)) { + Stat_t statbuf; + if (PerlLIO_lstat(s, &statbuf) < 0) tot--; + else if (S_ISDIR(statbuf.st_mode)) { SETERRNO(EISDIR, SS_NOPRIV); + tot--; } else { if (UNLINK(s)) diff --git a/perl.c b/perl.c index 1823c99..63d89c4 100644 --- a/perl.c +++ b/perl.c @@ -3840,16 +3840,13 @@ S_validate_suid(pTHX_ PerlIO *rsfp) if (my_euid != my_uid || my_egid != my_gid) { /* (suidperl doesn't exist, in fact) */ dVAR; int fd = PerlIO_fileno(rsfp); - if (fd < 0) { - Perl_croak(aTHX_ "Illegal suidscript"); - } else { - if (PerlLIO_fstat(fd, &PL_statbuf) < 0) { /* may be either wrapped or real suid */ - Perl_croak(aTHX_ "Illegal suidscript"); - } + Stat_t statbuf; + if (fd < 0 || PerlLIO_fstat(fd, &statbuf) < 0) { /* may be either wrapped or real suid */ + Perl_croak_nocontext( "Illegal suidscript"); } - if ((my_euid != my_uid && my_euid == PL_statbuf.st_uid && PL_statbuf.st_mode & S_ISUID) + if ((my_euid != my_uid && my_euid == statbuf.st_uid && statbuf.st_mode & S_ISUID) || - (my_egid != my_gid && my_egid == PL_statbuf.st_gid && PL_statbuf.st_mode & S_ISGID) + (my_egid != my_gid && my_egid == statbuf.st_gid && statbuf.st_mode & S_ISGID) ) if (!PL_do_undump) Perl_croak(aTHX_ "YOU HAVEN'T DISABLED SET-ID SCRIPTS IN THE KERNEL YET!\n\ diff --git a/pp_hot.c b/pp_hot.c index 1094510..dc484f7 100644 --- a/pp_hot.c +++ b/pp_hot.c @@ -1727,6 +1727,7 @@ Perl_do_readline(pTHX) XPUSHs(sv); if (type == OP_GLOB) { const char *t1; + Stat_t statbuf; if (SvCUR(sv) > 0 && SvCUR(PL_rs) > 0) { char * const tmps = SvEND(sv) - 1; @@ -1742,7 +1743,7 @@ Perl_do_readline(pTHX) if (strchr("$&*(){}[]'\";\\|?<>~`", *t1)) #endif break; - if (*t1 && PerlLIO_lstat(SvPVX_const(sv), &PL_statbuf) < 0) { + if (*t1 && PerlLIO_lstat(SvPVX_const(sv), &statbuf) < 0) { (void)POPs; /* Unmatched wildcard? Chuck it... */ continue; } diff --git a/pp_sys.c b/pp_sys.c index ebd675b..6010c06 100644 --- a/pp_sys.c +++ b/pp_sys.c @@ -3706,17 +3706,20 @@ PP(pp_rename) { dSP; dTARGET; int anum; +#ifndef HAS_RENAME + Stat_t statbuf; +#endif const char * const tmps2 = POPpconstx; const char * const tmps = SvPV_nolen_const(TOPs); TAINT_PROPER("rename"); #ifdef HAS_RENAME anum = PerlLIO_rename(tmps, tmps2); #else - if (!(anum = PerlLIO_stat(tmps, &PL_statbuf))) { + if (!(anum = PerlLIO_stat(tmps, &statbuf))) { if (same_dirent(tmps2, tmps)) /* can always rename to same name */ anum = 1; else { - if (PerlProc_geteuid() || PerlLIO_stat(tmps2, &PL_statbuf) < 0 || !S_ISDIR(PL_statbuf.st_mode)) + if (PerlProc_geteuid() || PerlLIO_stat(tmps2, &statbuf) < 0 || !S_ISDIR(statbuf.st_mode)) (void)UNLINK(tmps2); if (!(anum = link(tmps, tmps2))) anum = UNLINK(tmps); @@ -3878,7 +3881,8 @@ S_dooneliner(pTHX_ const char *cmd, const char *filename) return 0; } else { /* some mkdirs return no failure indication */ - anum = (PerlLIO_stat(save_filename, &PL_statbuf) >= 0); + Stat_t statbuf; + anum = (PerlLIO_stat(save_filename, &statbuf) >= 0); if (PL_op->op_type == OP_RMDIR) anum = !anum; if (anum) diff --git a/util.c b/util.c index 0a2c11e..049f997 100644 --- a/util.c +++ b/util.c @@ -3259,13 +3259,16 @@ Perl_find_script(pTHX_ const char *scriptname, bool dosearch, #endif DEBUG_p(PerlIO_printf(Perl_debug_log, "Looking for %s\n",cur)); - if (PerlLIO_stat(cur,&PL_statbuf) >= 0 - && !S_ISDIR(PL_statbuf.st_mode)) { - dosearch = 0; - scriptname = cur; + { + Stat_t statbuf; + if (PerlLIO_stat(cur,&statbuf) >= 0 + && !S_ISDIR(statbuf.st_mode)) { + dosearch = 0; + scriptname = cur; #ifdef SEARCH_EXTS - break; + break; #endif + } } #ifdef SEARCH_EXTS if (cur == scriptname) { @@ -3291,6 +3294,7 @@ Perl_find_script(pTHX_ const char *scriptname, bool dosearch, bufend = s + strlen(s); while (s < bufend) { + Stat_t statbuf; # ifdef DOSISH for (len = 0; *s && *s != ';'; len++, s++) { @@ -3327,8 +3331,8 @@ Perl_find_script(pTHX_ const char *scriptname, bool dosearch, do { #endif DEBUG_p(PerlIO_printf(Perl_debug_log, "Looking for %s\n",tmpbuf)); - retval = PerlLIO_stat(tmpbuf,&PL_statbuf); - if (S_ISDIR(PL_statbuf.st_mode)) { + retval = PerlLIO_stat(tmpbuf,&statbuf); + if (S_ISDIR(statbuf.st_mode)) { retval = -1; } #ifdef SEARCH_EXTS @@ -3339,10 +3343,10 @@ Perl_find_script(pTHX_ const char *scriptname, bool dosearch, #endif if (retval < 0) continue; - if (S_ISREG(PL_statbuf.st_mode) - && cando(S_IRUSR,TRUE,&PL_statbuf) + if (S_ISREG(statbuf.st_mode) + && cando(S_IRUSR,TRUE,&statbuf) #if !defined(DOSISH) - && cando(S_IXUSR,TRUE,&PL_statbuf) + && cando(S_IXUSR,TRUE,&statbuf) #endif ) { @@ -3353,11 +3357,16 @@ Perl_find_script(pTHX_ const char *scriptname, bool dosearch, xfailed = savepv(tmpbuf); } #ifndef DOSISH - if (!xfound && !seen_dot && !xfailed && - (PerlLIO_stat(scriptname,&PL_statbuf) < 0 - || S_ISDIR(PL_statbuf.st_mode))) + { + Stat_t statbuf; + if (!xfound && !seen_dot && !xfailed && + (PerlLIO_stat(scriptname,&statbuf) < 0 + || S_ISDIR(statbuf.st_mode))) +#endif + seen_dot = 1; /* Disable message. */ +#ifndef DOSISH + } #endif - seen_dot = 1; /* Disable message. */ if (!xfound) { if (flags & 1) { /* do or die? */ /* diag_listed_as: Can't execute %s */ -- 1.7.9.msysgit.0
RT-Send-CC: perl5-porters [...] perl.org
Download (untitled) / with headers
text/plain 536b
On Fri Aug 14 21:14:28 2015, bulk88 wrote: Show quoted text
> 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
Subject: Re: [perl #121351] Replace use of PL_statbuf and PL_timesbuf with auto variables
CC: perl5-porters [...] perl.org
To: bulk88 via RT <perlbug-followup [...] perl.org>
Date: Thu, 8 Oct 2015 16:57:02 +0100
From: Dave Mitchell <davem [...] iabyn.com>
Download (untitled) / with headers
text/plain 905b
On Fri, Sep 25, 2015 at 03:08:49PM -0700, bulk88 via RT wrote: Show quoted text
> 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.
RT-Send-CC: perl5-porters [...] perl.org
Download (untitled) / with headers
text/plain 1.4k
On Thu Oct 08 08:57:29 2015, davem wrote: Show quoted text
> 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
Date: Sun, 17 Jan 2016 15:25:31 +0000
From: Aaron Crane <arc [...] cpan.org>
Subject: Re: [perl #121351] Replace use of PL_statbuf and PL_timesbuf with auto variables
CC: Perl5 Porters <perl5-porters [...] perl.org>
To: James E Keenan via RT <perlbug-followup [...] perl.org>
Download (untitled) / with headers
text/plain 506b
bulk88 via RT <perlbug-followup@perl.org> wrote: Show quoted text
> 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/
Subject: Re: [perl #121351] Replace use of PL_statbuf and PL_timesbuf with auto variables
To: perl5-porters [...] perl.org
Date: Sun, 17 Jan 2016 15:54:58 +0000
From: ilmari [...] ilmari.org (Dagfinn Ilmari Mannsåker)
Download (untitled) / with headers
text/plain 776b
Aaron Crane <arc@cpan.org> writes: Show quoted text
> 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.
From c8879096cae5c2bfc008cf50ef582ab19595b9f6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dagfinn=20Ilmari=20Manns=C3=A5ker?= <ilmari@ilmari.org> Date: Sun, 17 Jan 2016 15:52:30 +0000 Subject: [PATCH] [perl #1369717] Remove non-doio.c uses of PL_statbuf These are the last remaining uses outside the interwoven mess in S_openn_cleanup and openn_setup in doio.c. --- ext/ODBM_File/ODBM_File.pm | 2 +- ext/ODBM_File/ODBM_File.xs | 3 ++- os2/os2.c | 5 +++-- 3 files changed, 6 insertions(+), 4 deletions(-) diff --git a/ext/ODBM_File/ODBM_File.pm b/ext/ODBM_File/ODBM_File.pm index 958232c..dd92fd3 100644 --- a/ext/ODBM_File/ODBM_File.pm +++ b/ext/ODBM_File/ODBM_File.pm @@ -7,7 +7,7 @@ require Tie::Hash; require XSLoader; our @ISA = qw(Tie::Hash); -our $VERSION = "1.12"; +our $VERSION = "1.13"; XSLoader::load(); diff --git a/ext/ODBM_File/ODBM_File.xs b/ext/ODBM_File/ODBM_File.xs index d1ece7f..f26abda 100644 --- a/ext/ODBM_File/ODBM_File.xs +++ b/ext/ODBM_File/ODBM_File.xs @@ -92,6 +92,7 @@ odbm_TIEHASH(dbtype, filename, flags, mode) { char *tmpbuf; void * dbp ; + struct stat statbuf; dMY_CXT; if (dbmrefcnt++) @@ -99,7 +100,7 @@ odbm_TIEHASH(dbtype, filename, flags, mode) Newx(tmpbuf, strlen(filename) + 5, char); SAVEFREEPV(tmpbuf); sprintf(tmpbuf,"%s.dir",filename); - if (stat(tmpbuf, &PL_statbuf) < 0) { + if (stat(tmpbuf, &statbuf) < 0) { if (flags & O_CREAT) { if (mode < 0 || close(creat(tmpbuf,mode)) < 0) croak("ODBM_File: Can't create %s", filename); diff --git a/os2/os2.c b/os2/os2.c index 8c5e941..a4f5015 100644 --- a/os2/os2.c +++ b/os2/os2.c @@ -1140,6 +1140,7 @@ do_spawn_ve(pTHX_ SV *really, U32 flag, U32 execf, char *inicmd, U32 addflag) if (!buf) buf = ""; /* XXX Needed? */ if (!buf[0]) { /* Empty... */ + struct stat statbuf; PerlIO_close(file); /* Special case: maybe from -Zexe build, so there is an executable around (contrary to @@ -1148,8 +1149,8 @@ do_spawn_ve(pTHX_ SV *really, U32 flag, U32 execf, char *inicmd, U32 addflag) reached this place). */ sv_catpv(scrsv, ".exe"); PL_Argv[0] = scr = SvPV(scrsv, n_a); /* Reload */ - if (PerlLIO_stat(scr,&PL_statbuf) >= 0 - && !S_ISDIR(PL_statbuf.st_mode)) { /* Found */ + if (PerlLIO_stat(scr,&statbuf) >= 0 + && !S_ISDIR(statbuf.st_mode)) { /* Found */ real_name = scr; pass++; goto reread; -- 2.7.0.rc3
Download (untitled) / with headers
text/plain 114b
-- "A disappointingly low fraction of the human race is, at any given time, on fire." - Stig Sandbeck Mathisen
From: ilmari [...] ilmari.org (Dagfinn Ilmari Mannsåker)
Date: Mon, 25 Jan 2016 15:29:33 +0000
To: perl5-porters [...] perl.org
Subject: Re: [perl #121351] Replace use of PL_statbuf and PL_timesbuf with auto variables
Download (untitled) / with headers
text/plain 811b
ilmari@ilmari.org (Dagfinn Ilmari Mannsåker) writes: Show quoted text
> 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
From: Zefram <zefram [...] fysh.org>
To: perl5-porters [...] perl.org
Subject: Re: [perl #121351] Replace use of PL_statbuf and PL_timesbuf with auto variables
Date: Mon, 13 Nov 2017 01:57:30 +0000
Download (untitled) / with headers
text/plain 158b
PL_statbuf was finally removed in 5.27.1, in commit 7e30e49f7461aeda1a5ab4539abfbe54f0f50e67. This ticket has been completed and can now be closed. -zefram
Download (untitled) / with headers
text/plain 317b
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.


This service is sponsored and maintained by Best Practical Solutions and runs on Perl.org infrastructure.

For issues related to this RT instance (aka "perlbug"), please contact perlbug-admin at perl.org