develooper Front page | perl.perl5.porters | Postings from December 2016

[perl #130122] [PATCH] Cleanup two argument open usage.

Thread Previous | Thread Next
From:
James E Keenan via RT
Date:
December 23, 2016 16:02
Subject:
[perl #130122] [PATCH] Cleanup two argument open usage.
Message ID:
rt-4.0.24-13165-1482508915-1461.130122-15-0@perl.org
On Fri, 23 Dec 2016 14:16:30 GMT, csjewell wrote:
> On Fri, 23 Dec 2016 05:46:03 -0800, jkeenan wrote:
> > On Thu, 17 Nov 2016 17:52:28 GMT, john@nixnuts.net wrote:
> > > This is a bug report for perl from john@nixnuts.net,
> > > generated with the help of perlbug 1.40 running under perl 5.25.7.
> > >
> > >
> > > -----------------------------------------------------------------
> > > The attached patch removes most two argument open() calls in the
> > > Perl codebase.
> > >
> > > The 'cpan' and 't' directories were skipped.
> > > Filehandle cloning was left as-is for 5.6.2 compatibility.
> > >
> > > Notable changes:
> > > - Storable
> > > - Devel::SelfStubber
> > > - Unicode::UCD
> > > - win32/bin/search.pl
> > >
> >
> > John, there are places in this patch where I believe we can do a bit
> > more cleanup, i.e., places where, after re-writing an 'open' call in
> > 3-arg form, we have double-quotes around a scalar holding a file
> > name.
> >
> > #####
> > $ grep -n -E '^\+.*?open[^"]+"\$' 130122-0001-Remove-most-usage-of-
> > the-two-argument-open.patch | grep -v -E 'or (die|return) "\$'
> > 232:+open(H,'>',"$file.new") || die "Cannot open $file.new:$!";
> > 1762:+    if (open(TMPC, '>', "$tmp.c")) {
> > 1771:+      open( CMDFILE, '>', "$tmp.com" );
> > 2408:+open (CODE, '>', "$tmp.c") || die "$iam: cannot create $tmp.c:
> > $!\n";
> > 2421:+    if (!open (INCLUDE,'<',"$LIB/$include")) {
> > 2434:+    open(H, '<', "$file.html") ||
> > 2443:+    open(H, '>', "$file.html") ||
> > 2461:+      open(my $in, '<', "$dir/$file") ||
> > 2479:+  open(SPLITOUT, '>', "$poddir/$file") ||
> > 3060:+open(OUT,'>',"${outdir}config.sh") || die "Can't open
> > ${outdir}config.sh: $!\n";
> > 3069:+if (open(SH,'<',"${outdir}config_h.SH")) {
> > 3078:+if (open(PL,'<',"${outdir}patchlevel.h")) {
> > 3091:+      open(MAN, '>', "$myname.3pl");
> > 3104:+    open (POD, '>', "$name.pod") || die "can't open $name.pod:
> > $!";
> > 3461:+  if (open(my $fh, '>', "$AppName.app")) {
> > 3470:+  if (open(my $fh, '>', "$AppName.rsc")) {
> > 4354:+  open my $fhdyna, '>', "$opts{distdir}/lib/Dynaloader.pm";
> > #####
> >
> > I believe that in these cases we don't need the double-quotes around
> > the 3rd arg -- correct?
> >
> > Thank you very much.
> 
> Incorrect. In all those cases, they are either appending an extension
> or filename on, or a second scalar. Therefore, they all need the
> double quotes, as an alternative to something like $AppName . '.app'

Ah!  The value of additional eyes on a problem. curtis++.

I've now reviewed the first 2000 lines of the patch.  Only 3500 to go!  Anyone want to help out?

Thank you very much.

-- 
James E Keenan (jkeenan@cpan.org)

---
via perlbug:  queue: perl5 status: open
https://rt.perl.org/Ticket/Display.html?id=130122

Thread Previous | Thread Next


nntp.perl.org: Perl Programming lists via nntp and http.
Comments to Ask Bjørn Hansen at ask@perl.org | Group listing | About