Skip Menu |
Report information
Id: 121351
Status: pending release
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


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