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

Filter::Simple with immediately following "no" #11853

Closed
p5pRT opened this issue Jan 7, 2012 · 20 comments
Closed

Filter::Simple with immediately following "no" #11853

p5pRT opened this issue Jan 7, 2012 · 20 comments

Comments

@p5pRT
Copy link

p5pRT commented Jan 7, 2012

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

Searchable as RT107726$

@p5pRT
Copy link
Author

p5pRT commented Jan 7, 2012

From user42@zip.com.au

With recent debian perl 5.14.2, if a Filter​::Simple module is turned off
with a "no" immediately after being turned on, like

  use MyFilter;
  no MyFilter;

then the remaining script after the "no" is ignored. The attached
foo.el script and MyFilter.pm module show the problem. Running

  perl foo.el

I expected it to print "this part is not filtered", but it doesn't. If
you put a blank line after "use MyFilter" it does.

I struck this with Smart​::Comments when I'd deleted everything between
use and no, but left those lines intending perhaps to add back some code
there shortly ...

Nosing around Filter​::Simple it looks like when the terminator is found
on the very first call of the filter_add() handler, the handler returns
0, which the perl parser perhaps takes to mean EOF, and therefore stops
reading.

I expect it ought to return with "no MyFilter" or suitably mangled
terminator, to have that run as code.

Dunno if an empty "use/no" block ought to be shown to the FILTER{} etc
code of the subclass. I suppose currently a "use MyFilter" immediately
followed by EOF has not called with an empty, so maybe no empties should
be shown to it, by default at least.

@p5pRT
Copy link
Author

p5pRT commented Jan 7, 2012

From user42@zip.com.au

foo.pl

@p5pRT
Copy link
Author

p5pRT commented Jan 7, 2012

From user42@zip.com.au

MyFilter.pm

@p5pRT
Copy link
Author

p5pRT commented Jan 7, 2012

From user42@zip.com.au



Flags​:
  category=library
  severity=medium
  module=Filter​::Simple


Site configuration information for perl 5.14.2​:

Configured by Debian Project at Mon Nov 28 21​:49​:25 UTC 2011.

Summary of my perl5 (revision 5 version 14 subversion 2) configuration​:
 
  Platform​:
  osname=linux, osvers=2.6.32-5-686, archname=i486-linux-gnu-thread-multi-64int
  uname='linux callisto 2.6.32-5-686 #1 smp mon oct 3 04​:15​:24 utc 2011 i686 gnulinux '
  config_args='-Dusethreads -Duselargefiles -Dccflags=-DDEBIAN -Dcccdlflags=-fPIC -Darchname=i486-linux-gnu -Dprefix=/usr -Dprivlib=/usr/share/perl/5.14 -Darchlib=/usr/lib/perl/5.14 -Dvendorprefix=/usr -Dvendorlib=/usr/share/perl5 -Dvendorarch=/usr/lib/perl5 -Dsiteprefix=/usr/local -Dsitelib=/usr/local/share/perl/5.14.2 -Dsitearch=/usr/local/lib/perl/5.14.2 -Dman1dir=/usr/share/man/man1 -Dman3dir=/usr/share/man/man3 -Dsiteman1dir=/usr/local/man/man1 -Dsiteman3dir=/usr/local/man/man3 -Duse64bitint -Dman1ext=1 -Dman3ext=3perl -Dpager=/usr/bin/sensible-pager -Uafs -Ud_csh -Ud_ualarm -Uusesfio -Uusenm -Ui_libutil -DDEBUGGING=-g -Doptimize=-O2 -Duseshrplib -Dlibperl=libperl.so.5.14.2 -des'
  hint=recommended, useposix=true, d_sigaction=define
  useithreads=define, usemultiplicity=define
  useperlio=define, d_sfio=undef, uselargefiles=define, usesocks=undef
  use64bitint=define, use64bitall=undef, uselongdouble=undef
  usemymalloc=n, bincompat5005=undef
  Compiler​:
  cc='cc', ccflags ='-D_REENTRANT -D_GNU_SOURCE -DDEBIAN -fno-strict-aliasing -pipe -fstack-protector -I/usr/local/include -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64',
  optimize='-O2 -g',
  cppflags='-D_REENTRANT -D_GNU_SOURCE -DDEBIAN -fno-strict-aliasing -pipe -fstack-protector -I/usr/local/include'
  ccversion='', gccversion='4.6.2', gccosandvers=''
  intsize=4, longsize=4, ptrsize=4, doublesize=8, byteorder=12345678
  d_longlong=define, longlongsize=8, d_longdbl=define, longdblsize=12
  ivtype='long long', ivsize=8, nvtype='double', nvsize=8, Off_t='off_t', lseeksize=8
  alignbytes=4, prototype=define
  Linker and Libraries​:
  ld='cc', ldflags =' -fstack-protector -L/usr/local/lib'
  libpth=/usr/local/lib /lib/i386-linux-gnu /lib/../lib /usr/lib/i386-linux-gnu /usr/lib/../lib /lib /usr/lib /usr/lib64
  libs=-lgdbm -lgdbm_compat -ldb -ldl -lm -lpthread -lc -lcrypt
  perllibs=-ldl -lm -lpthread -lc -lcrypt
  libc=, so=so, useshrplib=true, libperl=libperl.so.5.14.2
  gnulibc_version='2.13'
  Dynamic Linking​:
  dlsrc=dl_dlopen.xs, dlext=so, d_dlsymun=undef, ccdlflags='-Wl,-E'
  cccdlflags='-fPIC', lddlflags='-shared -O2 -g -L/usr/local/lib -fstack-protector'

Locally applied patches​:

@p5pRT
Copy link
Author

p5pRT commented Sep 6, 2013

From @rjbs

I have attempted to reproduce your problem in blead with the script you provided, but was
unable to.

  ~/tmp/FS$ cat foo.pl
  use lib '.';
  use MyFilter;
  no MyFilter;
  print "this part is not filtered\n";
  ~/tmp/FS$ cat MyFilter.pm
  package MyFilter;
  use Filter​::Simple;
  FILTER {
  print "FILTER code runs\n";
  }
  1;
  ~/tmp/FS$ perl foo.pl
  FILTER code runs
  this part is not filtered

I thought maybe the problem had been fixed since your report, but with 5.14.2, I still get the
same result.

  ~/tmp/FS$ perlbrew use 14.2
  ~/tmp/FS$ perl -v |head -2

  This is perl 5, version 14, subversion 2 (v5.14.2) built for darwin-2level
  ~/tmp/FS$ perl foo.pl
  FILTER code runs
  this part is not filtered

I am tentatively rejecting this bug. If you can provide further steps to reproduce it, please reply
and we can re-open it.

--
rjbs

@p5pRT
Copy link
Author

p5pRT commented Sep 6, 2013

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

@p5pRT
Copy link
Author

p5pRT commented Sep 6, 2013

@rjbs - Status changed from 'open' to 'rejected'

@p5pRT
Copy link
Author

p5pRT commented Sep 7, 2013

From user42@zip.com.au

"Ricardo SIGNES via RT" <perlbug-followup@​perl.org> writes​:

~/tmp/FS$ cat MyFilter.pm
package MyFilter;
use Filter​::Simple;
FILTER {
print "FILTER code runs\n";
}
1;

The MyFilter.pm must end as I posted without "1;". In that case it
still fails for me, with recent debian i386 5.14.2 and current filter
1.49 and Filter​::Simple 0.88. Such an end without "1;" is per the
examples in demo/*.pm.

--
If a grizzly bear is coming at you past advice has been "stand still".
Recently this has been revised to "run if you want to". Running is the
reason you have legs and during your last few seconds of life you may as
well put them to their intended use.

@p5pRT
Copy link
Author

p5pRT commented Sep 7, 2013

@rjbs - Status changed from 'rejected' to 'open'

@p5pRT
Copy link
Author

p5pRT commented Sep 7, 2013

From @rjbs

Whoops! Yes, verified.

So, the question I had was, then, "what is the magic true value?" It had the be the result of
calling FILTER. I hadn't included a semicolon after the subroutine passed to FILTER, so 1 became
its argument, making "1" the terminator for the filter, rather than the "no MyFilter" it expected.

I think the little state machine that gathers lines has an off-by-one, although I've really only
given it a cursory glance. I suggest this patch​:

Inline Patch
diff --git a/dist/Filter-Simple/lib/Filter/Simple.pm b/dist/Filter-Simple/lib/Filter/Simple.pm
index d1da0b2..f0ad0f3 100644
--- a/dist/Filter-Simple/lib/Filter/Simple.pm
+++ b/dist/Filter-Simple/lib/Filter/Simple.pm
@@ -198,6 +198,7 @@ sub gen_filter_import {
                     if ($terminator{terminator} &&
                         m/$terminator{terminator}/) {
                         $lastline = $_;
+                        $count++;
                         last;
                     }
                     $data .= $_;

Other eyes are encouraged.

--
rjbs

@p5pRT
Copy link
Author

p5pRT commented Sep 12, 2013

From user42@zip.com.au

"Ricardo SIGNES via RT" <perlbug-followup@​perl.org> writes​:

making "1" the terminator for the filter,

Ah, something subtle there.

I think the little state machine

It's actually a slurp through to the terminator, no? The docs could
make that clearer, since the bare filter stuff is normally line by line.

+ $count++;

Yes, looks likely. It actually only needs a flag rather than a count
anyway I suppose.

@p5pRT
Copy link
Author

p5pRT commented Sep 13, 2013

From @rjbs

* Kevin Ryde <user42@​zip.com.au> [2013-09-12T17​:57​:07]

Yes, looks likely. It actually only needs a flag rather than a count
anyway I suppose.

Could you write a test for the bug? Then I'll apply your test and my patch and
we'll high five. :)

--
rjbs

@p5pRT
Copy link
Author

p5pRT commented Sep 16, 2013

From user42@zip.com.au

"Ricardo Signes via RT" <perlbug-followup@​perl.org> writes​:

Could you write a test for the bug?

Yes.

I wouldn't mind also if the docs began by summarizing the good things of
Filter​::Simple rather than describing how hard is Filter​::Util​::Call
:-).

@p5pRT
Copy link
Author

p5pRT commented Sep 16, 2013

From @rjbs

* Kevin Ryde <user42@​zip.com.au> [2013-09-16T05​:01​:50]

"Ricardo Signes via RT" <perlbug-followup@​perl.org> writes​:

Could you write a test for the bug?

Yes.

I wouldn't mind also if the docs began by summarizing the good things of
Filter​::Simple rather than describing how hard is Filter​::Util​::Call
:-).

Such a patch would also be welcome! The days are clearly long gone when
anybody needs to be convinved to use Filter​::Simple rather than
filter​::Util​::Call. :)

--
rjbs

@p5pRT
Copy link
Author

p5pRT commented Aug 23, 2016

@dcollinsn - Status changed from 'open' to 'stalled'

@p5pRT
Copy link
Author

p5pRT commented Aug 26, 2016

From @cpansprout

On Thu Sep 12 19​:18​:12 2013, perl.p5p@​rjbs.manxome.org wrote​:

* Kevin Ryde <user42@​zip.com.au> [2013-09-12T17​:57​:07]

Yes, looks likely. It actually only needs a flag rather than a count
anyway I suppose.

Could you write a test for the bug? Then I'll apply your test and my
patch and
we'll high five. :)

I have just added a test in 7e9e80a and applied Ricardo’s patch as 8bc40f3.

--

Father Chrysostomos

@p5pRT
Copy link
Author

p5pRT commented Aug 26, 2016

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

@p5pRT
Copy link
Author

p5pRT commented Aug 26, 2016

@cpansprout - Status changed from 'open' to 'pending release'

@p5pRT
Copy link
Author

p5pRT commented May 30, 2017

From @khwilliamson

Thank you for filing this report. You have helped make Perl better.

With the release today of Perl 5.26.0, this and 210 other issues have been
resolved.

Perl 5.26.0 may be downloaded via​:
https://metacpan.org/release/XSAWYERX/perl-5.26.0

If you find that the problem persists, feel free to reopen this ticket.

@p5pRT
Copy link
Author

p5pRT commented May 30, 2017

@khwilliamson - Status changed from 'pending release' to 'resolved'

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

No branches or pull requests

1 participant