Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

h2ph using deprecated goto #10307

Closed
p5pRT opened this issue Apr 15, 2010 · 13 comments
Closed

h2ph using deprecated goto #10307

p5pRT opened this issue Apr 15, 2010 · 13 comments

Comments

@p5pRT
Copy link
Collaborator

@p5pRT p5pRT commented Apr 15, 2010

Migrated from rt.perl.org#74404 (status was 'resolved')

Searchable as RT74404$

@p5pRT
Copy link
Collaborator Author

@p5pRT p5pRT commented Apr 15, 2010

From @toddr

Created by @toddr

running h2ph produces warnings like this​:

Use of "goto" to jump into a construct is deprecated at utils/h2ph line 349, <IN> line xxxx

If it's still used, we'll need to clean this up. I lack the expertise to provide a patch on this one.

Perl Info

Flags:
    category=utilities
    severity=medium

Site configuration information for perl 5.12.0:

Configured by toddr at Wed Apr 14 16:37:04 CDT 2010.

Summary of my perl5 (revision 5 version 12 subversion 0) configuration:
   
  Platform:
    osname=linux, osvers=2.6.18-164.9.1.el5, archname=x86_64-linux
    uname='linux toddr-esx 2.6.18-164.9.1.el5 #1 smp tue dec 15 20:57:57 est 2009 x86_64 x86_64 x86_64 gnulinux '
    config_args='-des -Darchname=x86_64-linux -Dcc=gcc -DDEBUGGING=none -Doptimize=-Os -Dusemymalloc=y -Duseshrplib=true -Duselargefiles=yes -Duseposix=true -Dhint=recommended -Duseperlio=yes -Dccflags=-I/usr/local/perl/include -Wl,-rpath -Wl,/usr/local/perl/lib -L/usr/local/perl/lib -Dldflags=-Wl,-rpath -Wl,/usr/local/perl/lib -L/usr/local/perl/lib -Dprefix=/usr/local/perl -Dsiteprefix=/usr/local/perl -Dsitebin=/usr/local/perl/bin -Dsitelib=/usr/local/perl/lib/perl5/site_lib -Dsiteman1dir=/usr/local/perl/man/man1 -Dsiteman3dir=/usr/local/perl/man/man3 -Dcf_by=toddr -Dmyhostname=localhost -Dperladmin=root@localhost -Dcf_email=toddr@cpanel.net -Di_dbm=/usr/local/perl/include -Di_gdbm=/usr/local/perl/include -Di_ndbm=/usr/local/perl/include -Ud_dosuid -Uuserelocatableinc -Umad -Uusethreads -Uusemultiplicity -Uusesocks -Uuselongdouble -Acflags=-fPIC -DPIC -m64 -I/usr/local/perl/include -Aldflags=-L/usr/local/perl/lib -L/usr/lib64 -L/lib64 -Dlibpth=/usr/local/perl/lib /usr/local/lib64 /usr/local/lib /lib64 /usr/lib64  -Dlocincpth=/usr/local/perl/include /usr/local/include -Duse64bitint=yes -Duse64bitall=yes'
    hint=recommended, useposix=true, d_sigaction=define
    useithreads=undef, usemultiplicity=undef
    useperlio=define, d_sfio=undef, uselargefiles=define, usesocks=undef
    use64bitint=define, use64bitall=define, uselongdouble=undef
    usemymalloc=y, bincompat5005=undef
  Compiler:
    cc='gcc', ccflags ='-I/usr/local/perl/include -Wl,-rpath -Wl,/usr/local/perl/lib -L/usr/local/perl/lib -fno-strict-aliasing -pipe -fstack-protector -I/usr/local/perl/include -I/usr/local/include -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64',
    optimize='-Os',
    cppflags='-I/usr/local/perl/include -Wl,-rpath -Wl,/usr/local/perl/lib -L/usr/local/perl/lib -fno-strict-aliasing -pipe -fstack-protector -I/usr/local/perl/include -I/usr/local/include'
    ccversion='', gccversion='4.1.2 20080704 (Red Hat 4.1.2-46)', gccosandvers=''
    intsize=4, longsize=8, ptrsize=8, doublesize=8, byteorder=12345678
    d_longlong=define, longlongsize=8, d_longdbl=define, longdblsize=16
    ivtype='long', ivsize=8, nvtype='double', nvsize=8, Off_t='off_t', lseeksize=8
    alignbytes=8, prototype=define
  Linker and Libraries:
    ld='gcc', ldflags ='-Wl,-rpath -Wl,/usr/local/perl/lib -L/usr/local/perl/lib -L/usr/local/perl/lib -L/usr/lib64 -L/lib64 -fstack-protector -L/usr/local/lib'
    libpth=/usr/local/perl/lib /usr/local/lib64 /usr/local/lib /lib64 /usr/lib64
    libs=-lnsl -lgdbm -ldb -ldl -lm -lcrypt -lutil -lc -lgdbm_compat
    perllibs=-lnsl -ldl -lm -lcrypt -lutil -lc
    libc=/lib/libc-2.5.so, so=so, useshrplib=true, libperl=libperl.so
    gnulibc_version='2.5'
  Dynamic Linking:
    dlsrc=dl_dlopen.xs, dlext=so, d_dlsymun=undef, ccdlflags='-Wl,-E -Wl,-rpath,/usr/local/perl/lib/perl5/5.12.0/x86_64-linux/CORE'
    cccdlflags='-fPIC', lddlflags='-shared -Os -L/usr/local/perl/lib -L/usr/lib64 -L/lib64 -L/usr/local/lib -fstack-protector'


@INC for perl 5.12.0:
    /usr/local/perl/lib/perl5/site_lib/x86_64-linux
    /usr/local/perl/lib/perl5/site_lib
    /usr/local/perl/lib/perl5/5.12.0/x86_64-linux
    /usr/local/perl/lib/perl5/5.12.0
    .


Environment for perl 5.12.0:
    HOME=/home/toddr
    LANG=en_US.UTF-8
    LANGUAGE (unset)
    LD_LIBRARY_PATH (unset)
    LOGDIR (unset)
    PATH=/usr/local/perl/bin:/usr/local/bin:/usr/bin:/bin:/usr/sbin:/sbin:/usr/local/bin:/usr/X11/bin:/home/toddr/scripts:
    PERL_BADLANG (unset)
    SHELL=/bin/zsh


@p5pRT
Copy link
Collaborator Author

@p5pRT p5pRT commented Apr 16, 2010

From zefram@fysh.org

Todd Rinaldo wrote​:

Use of "goto" to jump into a construct is deprecated at utils/h2ph line 349, <IN> line xxxx

You could factor out the EMIT code into a subroutine. Call it from both
the current "EMIT​:" and "goto EMIT" sites.

-zefram

@p5pRT
Copy link
Collaborator Author

@p5pRT p5pRT commented Apr 16, 2010

The RT System itself - Status changed from 'new' to 'open'

@p5pRT
Copy link
Collaborator Author

@p5pRT p5pRT commented Apr 16, 2010

From @toddr

On Apr 16, 2010, at 6​:02 AM, Zefram wrote​:

Todd Rinaldo wrote​:

Use of "goto" to jump into a construct is deprecated at utils/h2ph line 349, <IN> line xxxx

You could factor out the EMIT code into a subroutine. Call it from both
the current "EMIT​:" and "goto EMIT" sites.

It looks like h2ph makes copious use of global variables. As a result, I can commit this evil for a minimal patch. No code was changed moving it into the sub other than passing $proto, which is the only non-global used (but not modified).

I tried a 5.12.0 build with this patch and it seems to be passing tests. there is a lib/h2ph.t file but I don't know what it tests exactly.

@p5pRT
Copy link
Collaborator Author

@p5pRT p5pRT commented Apr 16, 2010

From @toddr

Inline Patch
diff --git a/utils/h2ph.PL b/utils/h2ph.PL
index 8f56db4..3d2fe85 100644
--- a/utils/h2ph.PL
+++ b/utils/h2ph.PL
@@ -147,23 +147,7 @@ while (defined (my $file = next_file())) {
 		    s/^\s+//;
 		    expr();
 		    $new =~ s/(["\\])/\\$1/g;       #"]);
-		  EMIT:
-		    $new = reindent($new);
-		    $args = reindent($args);
-		    if ($t ne '') {
-			$new =~ s/(['\\])/\\$1/g;   #']);
-			if ($opt_h) {
-			    print OUT $t,
-                            "eval \"\\n#line $eval_index $outfile\\n\" . 'sub $name $proto\{\n$t    ${args}eval q($new);\n$t}' unless defined(\&$name);\n";
-                            $eval_index++;
-			} else {
-			    print OUT $t,
-                            "eval 'sub $name $proto\{\n$t    ${args}eval q($new);\n$t}' unless defined(\&$name);\n";
-			}
-		    } else {
-                      print OUT "unless(defined(\&$name)) {\n    sub $name $proto\{\n\t${args}eval q($new);\n    }\n}\n";
-		    }
-		    %curargs = ();
+		    EMIT($proto);
 		} else {
 		    s/^\s+//;
 		    expr();
@@ -380,7 +364,7 @@ while (defined (my $file = next_file())) {
 	    $new =~ s/&$_\b/\$$_/g for @local_variables;
 	    $new =~ s/(["\\])/\\$1/g;       #"]);
 	    # now that's almost like a macro (we hope)
-	    goto EMIT;
+	    EMIT($proto);
 	}
     }
     $Is_converted{$file} = 1;
@@ -400,6 +384,28 @@ if ($opt_e && (scalar(keys %bad_file) > 0)) {
 
 exit $Exit;
 
+sub EMIT {
+    my $proto = shift;
+    
+    $new = reindent($new);
+    $args = reindent($args);
+    if ($t ne '') {
+    $new =~ s/(['\\])/\\$1/g;   #']);
+    if ($opt_h) {
+        print OUT $t,
+                    "eval \"\\n#line $eval_index $outfile\\n\" . 'sub $name $proto\{\n$t    ${args}eval q($new);\n$t}' unless defined(\&$name);\n";
+                    $eval_index++;
+    } else {
+        print OUT $t,
+                    "eval 'sub $name $proto\{\n$t    ${args}eval q($new);\n$t}' unless defined(\&$name);\n";
+    }
+    } else {
+              print OUT "unless(defined(\&$name)) {\n    sub $name $proto\{\n\t${args}eval q($new);\n    }\n}\n";
+    }
+    %curargs = ();
+    return;
+}
+
 sub expr {
     $new = '"(assembly code)"' and return if /\b__asm__\b/; # freak out.
     my $joined_args;
@p5pRT
Copy link
Collaborator Author

@p5pRT p5pRT commented Aug 31, 2010

From @toddr

A patch was added to the ticket to get this fixed. Could it please be added to blead?

http​://rt.perl.org/rt3/Ticket/Attachment/683450/325408/patch.txt

Thanks,
Todd

@p5pRT
Copy link
Collaborator Author

@p5pRT p5pRT commented Sep 1, 2010

From @rafl

Todd Rinaldo <toddr@​cpanel.net> writes​:

A patch was added to the ticket to get this fixed. Could it please be
added to blead?

That patch doesn't apply against blead. Could you please prepare a new
patch, preferably in git-format-patch format (so it'll contain both a
commit message and proper credit for your work) as well as without
adding more trailing whitespace?

Thanks!

@p5pRT
Copy link
Collaborator Author

@p5pRT p5pRT commented Sep 1, 2010

From @toddr

The patch differences are accidental and nothing to do with my change. A fuzz 0 patch is
attached.

@p5pRT
Copy link
Collaborator Author

@p5pRT p5pRT commented Sep 1, 2010

From @toddr

Inline Patch
diff --git a/utils/h2ph.PL b/utils/h2ph.PL
index 1255807..d8efcc0 100644
--- a/utils/h2ph.PL
+++ b/utils/h2ph.PL
@@ -147,23 +147,7 @@ while (defined (my $file = next_file())) {
 		    s/^\s+//;
 		    expr();
 		    $new =~ s/(["\\])/\\$1/g;       #"]);
-		  EMIT:
-		    $new = reindent($new);
-		    $args = reindent($args);
-		    if ($t ne '') {
-			$new =~ s/(['\\])/\\$1/g;   #']);
-			if ($opt_h) {
-			    print OUT $t,
-                            "eval \"\\n#line $eval_index $outfile\\n\" . 'sub $name $proto\{\n$t    ${args}eval q($new);\n$t}' unless defined(\&$name);\n";
-                            $eval_index++;
-			} else {
-			    print OUT $t,
-                            "eval 'sub $name $proto\{\n$t    ${args}eval q($new);\n$t}' unless defined(\&$name);\n";
-			}
-		    } else {
-                      print OUT "unless(defined(\&$name)) {\n    sub $name $proto\{\n\t${args}eval q($new);\n    }\n}\n";
-		    }
-		    %curargs = ();
+		    EMIT($proto);
 		} else {
 		    s/^\s+//;
 		    expr();
@@ -380,7 +364,7 @@ while (defined (my $file = next_file())) {
 	    $new =~ s/&$_\b/\$$_/g for @local_variables;
 	    $new =~ s/(["\\])/\\$1/g;       #"]);
 	    # now that's almost like a macro (we hope)
-	    goto EMIT;
+	    EMIT($proto);
 	}
     }
     $Is_converted{$file} = 1;
@@ -400,6 +384,28 @@ if ($opt_e && (scalar(keys %bad_file) > 0)) {
 
 exit $Exit;
 
+sub EMIT {
+    my $proto = shift;
+    
+    $new = reindent($new);
+    $args = reindent($args);
+    if ($t ne '') {
+    $new =~ s/(['\\])/\\$1/g;   #']);
+    if ($opt_h) {
+        print OUT $t,
+                    "eval \"\\n#line $eval_index $outfile\\n\" . 'sub $name $proto\{\n$t    ${args}eval q($new);\n$t}' unless defined(\&$name);\n";
+                    $eval_index++;
+    } else {
+        print OUT $t,
+                    "eval 'sub $name $proto\{\n$t    ${args}eval q($new);\n$t}' unless defined(\&$name);\n";
+    }
+    } else {
+              print OUT "unless(defined(\&$name)) {\n    sub $name $proto\{\n\t${args}eval q($new);\n    }\n}\n";
+    }
+    %curargs = ();
+    return;
+}
+
 sub expr {
     if (/\b__asm__\b/) {	# freak out
 	$new = '"(assembly code)"';
@p5pRT
Copy link
Collaborator Author

@p5pRT p5pRT commented Sep 1, 2010

From @toddr

Providing proper p5p patch.

@p5pRT
Copy link
Collaborator Author

@p5pRT p5pRT commented Sep 1, 2010

From @toddr

From 9114805349ec4b13b9fe13c3ae89dc22161b2210 Mon Sep 17 00​:00​:00 2001
From​: Todd Rinaldo <toddr@​cpan.org>
Date​: Wed, 1 Sep 2010 11​:05​:26 -0500
Subject​: [PATCH 21/21] RT 74404 - h2ph using deprecated goto

It looks like h2ph makes copious use of global variables. As a result, I can
commit this evil for a minimal patch. No code was changed moving it into the
sub other than passing $proto, which is the only non-global used (but not
modified).

I tried a 5.12.0 build with this patch and it seems to be passing tests.
There is a lib/h2ph.t file but I don't know what it tests exactly.


utils/h2ph.PL | 42 ++++++++++++++++++++++++------------------
1 files changed, 24 insertions(+), 18 deletions(-)

Inline Patch
diff --git a/utils/h2ph.PL b/utils/h2ph.PL
index 1255807..d8efcc0 100644
--- a/utils/h2ph.PL
+++ b/utils/h2ph.PL
@@ -147,23 +147,7 @@ while (defined (my $file = next_file())) {
 		    s/^\s+//;
 		    expr();
 		    $new =~ s/(["\\])/\\$1/g;       #"]);
-		  EMIT:
-		    $new = reindent($new);
-		    $args = reindent($args);
-		    if ($t ne '') {
-			$new =~ s/(['\\])/\\$1/g;   #']);
-			if ($opt_h) {
-			    print OUT $t,
-                            "eval \"\\n#line $eval_index $outfile\\n\" . 'sub $name $proto\{\n$t    ${args}eval q($new);\n$t}' unless defined(\&$name);\n";
-                            $eval_index++;
-			} else {
-			    print OUT $t,
-                            "eval 'sub $name $proto\{\n$t    ${args}eval q($new);\n$t}' unless defined(\&$name);\n";
-			}
-		    } else {
-                      print OUT "unless(defined(\&$name)) {\n    sub $name $proto\{\n\t${args}eval q($new);\n    }\n}\n";
-		    }
-		    %curargs = ();
+		    EMIT($proto);
 		} else {
 		    s/^\s+//;
 		    expr();
@@ -380,7 +364,7 @@ while (defined (my $file = next_file())) {
 	    $new =~ s/&$_\b/\$$_/g for @local_variables;
 	    $new =~ s/(["\\])/\\$1/g;       #"]);
 	    # now that's almost like a macro (we hope)
-	    goto EMIT;
+	    EMIT($proto);
 	}
     }
     $Is_converted{$file} = 1;
@@ -400,6 +384,28 @@ if ($opt_e && (scalar(keys %bad_file) > 0)) {
 
 exit $Exit;
 
+sub EMIT {
+    my $proto = shift;
+    
+    $new = reindent($new);
+    $args = reindent($args);
+    if ($t ne '') {
+    $new =~ s/(['\\])/\\$1/g;   #']);
+    if ($opt_h) {
+        print OUT $t,
+                    "eval \"\\n#line $eval_index $outfile\\n\" . 'sub $name $proto\{\n$t    ${args}eval q($new);\n$t}' unless defined(\&$name);\n";
+                    $eval_index++;
+    } else {
+        print OUT $t,
+                    "eval 'sub $name $proto\{\n$t    ${args}eval q($new);\n$t}' unless defined(\&$name);\n";
+    }
+    } else {
+              print OUT "unless(defined(\&$name)) {\n    sub $name $proto\{\n\t${args}eval q($new);\n    }\n}\n";
+    }
+    %curargs = ();
+    return;
+}
+
 sub expr {
     if (/\b__asm__\b/) {	# freak out
 	$new = '"(assembly code)"';
-- 
1.7.2
@p5pRT
Copy link
Collaborator Author

@p5pRT p5pRT commented Sep 1, 2010

From @rafl

Applied to blead, along with a test. Thank you!

@p5pRT
Copy link
Collaborator Author

@p5pRT p5pRT commented Sep 1, 2010

@rafl - Status changed from 'open' to 'resolved'

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
1 participant
You can’t perform that action at this time.