Perl / perl5

🐪 The Perl programming language
https://dev.perl.org/perl5/
Other
1.91k stars 542 forks source link

tell/seek misbehave for :crlf layer for perl-5.8.0 #5813

Closed p5pRT closed 21 years ago

p5pRT commented 22 years ago

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

Searchable as RT15986$

p5pRT commented 22 years ago

From vkonovalov@peterstar.ru

Created by vkonovalov@peterstar.ru

In some cases seek/tell misbehave for :crlf layer. Following sample program demonstrates that problem​: it tries tell and seek function and\, in case without :crlf layer (2nd one) it behaves correctly\, whereas same logic with :crlf layer seem to misbehave\, because seek returns into another location.

use 5.008;

my $fh; my $fcontents = join ""\, map {"$_\r\n"} "a".."zzz";

open $fh\, "\<​:crlf"\, \$fcontents; #open $fh\, "\<"\, \$fcontents; $/ = "xxx"; $_ = \<$fh>; $pos = tell $fh; # pos must be behind "xxx"\, before "\nyyy\n" seek $fh\, $pos\, 0; $/ = "\n"; $s = \<$fh>.\<$fh>; print "not okay​:[$s]\n";

open $fh\, "\<"\, \$fcontents; $/ = "xxx"; $_ = \<$fh>; $pos = tell $fh; # pos must be behind "xxx"\, before "\nyyy\n" seek $fh\, $pos\, 0; $/ = "\n"; $s = \<$fh>.\<$fh>; print "okay​:[$s]\n";

Same behaviour on Win32\, perl-5.8.0 built with MSVC++\, and with BC++. Also\, cygwin build seems to have such problem as well.

Perl Info ``` Flags: category=core severity=medium Site configuration information for perl v5.8.0: Configured by Administrator at Sat Jul 20 19:24:22 2002. Summary of my perl5 (revision 5 version 8 subversion 0) configuration: Platform: osname=MSWin32, osvers=4.0, archname=MSWin32-x86-multi-thread uname='' config_args='undef' hint=recommended, useposix=true, d_sigaction=undef usethreads=undef use5005threads=undef useithreads=define usemultiplicity=define useperlio=define d_sfio=undef uselargefiles=undef usesocks=undef use64bitint=undef use64bitall=undef uselongdouble=undef usemymalloc=n, bincompat5005=undef Compiler: cc='cl', ccflags ='-nologo -Gf -W3 -MD -DNDEBUG -O1 -DWIN32 -D_CONSOLE -DNO_STRICT -DPERL_I MPLICIT_CONTEXT -DPERL_IMPLICIT_SYS -DUSE_PERLIO -DPERL_MSVCRT_READFIX', optimize='-MD -DNDEBUG -O1', cppflags='-DWIN32' ccversion='', gccversion='', gccosandvers='' intsize=4, longsize=4, ptrsize=4, doublesize=8, byteorder=1234 d_longlong=undef, longlongsize=8, d_longdbl=define, longdblsize=10 ivtype='long', ivsize=4, nvtype='double', nvsize=8, Off_t='off_t', lseeksize=4 alignbytes=8, prototype=define Linker and Libraries: ld='link', ldflags '-nologo -nodefaultlib -release -libpath:"d:\perl58m\lib\CORE" -machine:x8 6' libpth=D:\MSVStudio\VC98\lib libs= oldnames.lib kernel32.lib user32.lib gdi32.lib winspool.lib comdlg32.lib advapi32.lib shell32.lib ole32.lib oleaut32.lib netapi32.lib uuid.lib wsock32.lib mpr.lib winmm.lib version.lib odbc32.lib odbccp32.lib msvcrt.lib perllibs= oldnames.lib kernel32.lib user32.lib gdi32.lib winspool.lib comdlg32.lib advapi32.lib shell32.lib ole32.lib oleaut32.lib netapi32.lib uuid.lib wsock32.lib mpr.lib winmm.lib version.lib odbc32.lib odbccp32.lib msvcrt.lib libc=msvcrt.lib, so=dll, useshrplib=yes, libperl=perl58.lib gnulibc_version='undef' Dynamic Linking: dlsrc=dl_win32.xs, dlext=dll, d_dlsymun=undef, ccdlflags=' ' cccdlflags=' ', ddlflags='-dll -nologo -nodefaultlib -release -libpath:"d:\perl58m\lib\CORE " -machine:x86' Locally applied patches: @INC for perl v5.8.0: D:\Work\PerlScripts\utl D:\Work\PerlScripts\translations D:\Work\PerlScripts\sgml D:\Work\PerlScripts\pleps d:/perl58m/lib d:/perl58m/site/lib . Environment for perl v5.8.0: HOME=d:\apps LANG (unset) LANGUAGE (unset) LD_LIBRARY_PATH (unset) LOGDIR (unset) PATH=d:\perl58m\bin;d:\sh;d:\perl561\bin;d:\apps\vim;d:\apps\vslick-40\win;d :\bin;d:\cygwin\bin;C:\WINNT\system32;C:\WINNT;C:\WINNT\System32\Wbem;d:\bor land\cbuilder5\bin PERL5CFG=d:\apps PERL5LIB=D:\Work\PerlScripts\utl;D:\Work\PerlScripts\translations;D:\Work\Pe rlScripts\sgml;D:\Work\PerlScripts\pleps PERL_BADLANG (unset) SHELL (unset) ```
p5pRT commented 22 years ago

From vkonovalov@spb.lucent.com

Dear all!

After some investigations I probably found a reason of misbehaviour which\, to say\, is occured not only on Win32\, but should misbehave everywhere.

Patch to t/io/crlf.t introduces a test which will fail on 5.8.0\, and patch to perlio.c should fix it. Actually "posn" struct number is "--"-ed in some code\, so I "++" it to "undo" counter mismatch.

Nick\, could you please see if I'm correct?

Best wishes\, Vadim.

-----Original Message----- From​: Vadim Konovalov [mailto​:perlbug@​perl.org] Sent​: Monday\, August 05\, 2002 9​:17 PM To​: perl5-porters@​perl.org Subject​: [perl #15986] tell/seek misbehave for :crlf layer for perl-5.8.0

# New Ticket Created by "Vadim Konovalov" # Please include the string​: [perl #15986] # in the subject line of all future correspondence about this issue. # \<URL​: http​://rt.perl.org/rt2/Ticket/Display.html?id=15986 >

This is a bug report for perl from vkonovalov@​peterstar.ru\, generated with the help of perlbug 1.34 running under perl v5.8.0.

----------------------------------------------------------------- [Please enter your report here]

In some cases seek/tell misbehave for :crlf layer. Following sample program demonstrates that problem​: it tries tell and seek function and\, in case without :crlf layer (2nd one) it behaves correctly\, whereas same logic with :crlf layer seem to misbehave\, because seek returns into another location.

use 5.008;

my $fh; my $fcontents = join ""\, map {"$_\r\n"} "a".."zzz";

open $fh\, "\<​:crlf"\, \$fcontents; #open $fh\, "\<"\, \$fcontents; $/ = "xxx"; $_ = \<$fh>; $pos = tell $fh; # pos must be behind "xxx"\, before "\nyyy\n" seek $fh\, $pos\, 0; $/ = "\n"; $s = \<$fh>.\<$fh>; print "not okay​:[$s]\n";

open $fh\, "\<"\, \$fcontents; $/ = "xxx"; $_ = \<$fh>; $pos = tell $fh; # pos must be behind "xxx"\, before "\nyyy\n" seek $fh\, $pos\, 0; $/ = "\n"; $s = \<$fh>.\<$fh>; print "okay​:[$s]\n";

Same behaviour on Win32\, perl-5.8.0 built with MSVC++\, and with BC++. Also\, cygwin build seems to have such problem as well.

[Please do not change anything below this line] ----------------------------------------------------------------- --- Flags​: category=core severity=medium --- Site configuration information for perl v5.8.0​:

Configured by Administrator at Sat Jul 20 19​:24​:22 2002.

Summary of my perl5 (revision 5 version 8 subversion 0) configuration​: Platform​: osname=MSWin32\, osvers=4.0\, archname=MSWin32-x86-multi-thread uname='' config_args='undef' hint=recommended\, useposix=true\, d_sigaction=undef usethreads=undef use5005threads=undef useithreads=define usemultiplicity=define useperlio=define d_sfio=undef uselargefiles=undef usesocks=undef use64bitint=undef use64bitall=undef uselongdouble=undef usemymalloc=n\, bincompat5005=undef Compiler​: cc='cl'\, ccflags ='-nologo -Gf -W3 -MD -DNDEBUG -O1 -DWIN32 -D_CONSOLE -DNO_STRICT -DPERL_I MPLICIT_CONTEXT -DPERL_IMPLICIT_SYS -DUSE_PERLIO -DPERL_MSVCRT_READFIX'\, optimize='-MD -DNDEBUG -O1'\, cppflags='-DWIN32' ccversion=''\, gccversion=''\, gccosandvers='' intsize=4\, longsize=4\, ptrsize=4\, doublesize=8\, byteorder=1234 d_longlong=undef\, longlongsize=8\, d_longdbl=define\, longdblsize=10 ivtype='long'\, ivsize=4\, nvtype='double'\, nvsize=8\, Off_t='off_t'\, lseeksize=4 alignbytes=8\, prototype=define Linker and Libraries​: ld='link'\, ldflags '-nologo -nodefaultlib -release
-libpath​:"d​:\perl58m\lib\CORE" -machine​:x8 6' libpth=D​:\MSVStudio\VC98\lib libs= oldnames.lib kernel32.lib user32.lib gdi32.lib winspool.lib comdlg32.lib advapi32.lib shell32.lib ole32.lib oleaut32.lib
netapi32.lib uuid.lib wsock32.lib mpr.lib winmm.lib version.lib odbc32.lib odbccp32.lib msvcrt.lib perllibs= oldnames.lib kernel32.lib user32.lib gdi32.lib winspool.lib comdlg32.lib advapi32.lib shell32.lib ole32.lib oleaut32.lib
netapi32.lib uuid.lib wsock32.lib mpr.lib winmm.lib version.lib odbc32.lib odbccp32.lib msvcrt.lib libc=msvcrt.lib\, so=dll\, useshrplib=yes\, libperl=perl58.lib gnulibc_version='undef' Dynamic Linking​: dlsrc=dl_win32.xs\, dlext=dll\, d_dlsymun=undef\, ccdlflags=' ' cccdlflags=' '\, ddlflags='-dll -nologo -nodefaultlib -release
-libpath​:"d​:\perl58m\lib\CORE " -machine​:x86'

Locally applied patches​:

--- @​INC for perl v5.8.0​: D​:\Work\PerlScripts\utl D​:\Work\PerlScripts\translations D​:\Work\PerlScripts\sgml D​:\Work\PerlScripts\pleps d​:/perl58m/lib d​:/perl58m/site/lib .

--- Environment for perl v5.8.0​: HOME=d​:\apps LANG (unset) LANGUAGE (unset) LD_LIBRARY_PATH (unset) LOGDIR (unset)

PATH=d​:\perl58m\bin;d​:\sh;d​:\perl561\bin;d​:\apps\vim;d​:\apps\v slick-40\win;d :\bin;d​:\cygwin\bin;C​:\WINNT\system32;C​:\WINNT;C​:\WINNT\System 32\Wbem;d​:\bor land\cbuilder5\bin PERL5CFG=d​:\apps

PERL5LIB=D​:\Work\PerlScripts\utl;D​:\Work\PerlScripts\translati ons;D​:\Work\Pe rlScripts\sgml;D​:\Work\PerlScripts\pleps PERL_BADLANG (unset) SHELL (unset)

p5pRT commented 22 years ago

From vkonovalov@spb.lucent.com

crlf.t.diff

p5pRT commented 22 years ago

From vkonovalov@spb.lucent.com

perlio.c.diff

p5pRT commented 22 years ago

From vkonovalov@peterstar.ru

here are same patches but a bit improved.

After some investigations I probably found a reason of misbehaviour which\, to say\, is occured not only on Win32\, but should misbehave everywhere.

Patch to t/io/crlf.t introduces a test which will fail on 5.8.0\, and patch to perlio.c should fix it. Actually "posn" struct number is "--"-ed in some code\, so I "++" it to "undo" counter mismatch.

Nick\, could you please see if I'm correct?

Best wishes\, Vadim.

-----Original Message----- From​: Vadim Konovalov [mailto​:perlbug@​perl.org] Sent​: Monday\, August 05\, 2002 9​:17 PM To​: perl5-porters@​perl.org Subject​: [perl #15986] tell/seek misbehave for :crlf layer for perl-5.8.0

# New Ticket Created by "Vadim Konovalov" # Please include the string​: [perl #15986] # in the subject line of all future correspondence about this issue. # \<URL​: http​://rt.perl.org/rt2/Ticket/Display.html?id=15986 >

This is a bug report for perl from vkonovalov@​peterstar.ru\, generated with the help of perlbug 1.34 running under perl v5.8.0.

----------------------------------------------------------------- [Please enter your report here]

In some cases seek/tell misbehave for :crlf layer. Following sample program demonstrates that problem​: it tries tell and seek function and\, in case without :crlf layer (2nd one) it behaves correctly\, whereas same logic with :crlf layer seem to misbehave\, because seek returns into another location.

use 5.008;

my $fh; my $fcontents = join ""\, map {"$_\r\n"} "a".."zzz";

open $fh\, "\<​:crlf"\, \$fcontents; #open $fh\, "\<"\, \$fcontents; $/ = "xxx"; $_ = \<$fh>; $pos = tell $fh; # pos must be behind "xxx"\, before "\nyyy\n" seek $fh\, $pos\, 0; $/ = "\n"; $s = \<$fh>.\<$fh>; print "not okay​:[$s]\n";

open $fh\, "\<"\, \$fcontents; $/ = "xxx"; $_ = \<$fh>; $pos = tell $fh; # pos must be behind "xxx"\, before "\nyyy\n" seek $fh\, $pos\, 0; $/ = "\n"; $s = \<$fh>.\<$fh>; print "okay​:[$s]\n";

Same behaviour on Win32\, perl-5.8.0 built with MSVC++\, and with BC++. Also\, cygwin build seems to have such problem as well.

[Please do not change anything below this line] ----------------------------------------------------------------- --- Flags​: category=core severity=medium --- Site configuration information for perl v5.8.0​:

Configured by Administrator at Sat Jul 20 19​:24​:22 2002.

Summary of my perl5 (revision 5 version 8 subversion 0) configuration​: Platform​: osname=MSWin32\, osvers=4.0\, archname=MSWin32-x86-multi-thread uname='' config_args='undef' hint=recommended\, useposix=true\, d_sigaction=undef usethreads=undef use5005threads=undef useithreads=define usemultiplicity=define useperlio=define d_sfio=undef uselargefiles=undef usesocks=undef use64bitint=undef use64bitall=undef uselongdouble=undef usemymalloc=n\, bincompat5005=undef Compiler​: cc='cl'\, ccflags ='-nologo -Gf -W3 -MD -DNDEBUG -O1 -DWIN32 -D_CONSOLE -DNO_STRICT -DPERL_I MPLICIT_CONTEXT -DPERL_IMPLICIT_SYS -DUSE_PERLIO -DPERL_MSVCRT_READFIX'\, optimize='-MD -DNDEBUG -O1'\, cppflags='-DWIN32' ccversion=''\, gccversion=''\, gccosandvers='' intsize=4\, longsize=4\, ptrsize=4\, doublesize=8\, byteorder=1234 d_longlong=undef\, longlongsize=8\, d_longdbl=define\, longdblsize=10 ivtype='long'\, ivsize=4\, nvtype='double'\, nvsize=8\, Off_t='off_t'\, lseeksize=4 alignbytes=8\, prototype=define Linker and Libraries​: ld='link'\, ldflags '-nologo -nodefaultlib -release -libpath​:"d​:\perl58m\lib\CORE" -machine​:x8 6' libpth=D​:\MSVStudio\VC98\lib libs= oldnames.lib kernel32.lib user32.lib gdi32.lib winspool.lib comdlg32.lib advapi32.lib shell32.lib ole32.lib oleaut32.lib netapi32.lib uuid.lib wsock32.lib mpr.lib winmm.lib version.lib odbc32.lib odbccp32.lib msvcrt.lib perllibs= oldnames.lib kernel32.lib user32.lib gdi32.lib winspool.lib comdlg32.lib advapi32.lib shell32.lib ole32.lib oleaut32.lib netapi32.lib uuid.lib wsock32.lib mpr.lib winmm.lib version.lib odbc32.lib odbccp32.lib msvcrt.lib libc=msvcrt.lib\, so=dll\, useshrplib=yes\, libperl=perl58.lib gnulibc_version='undef' Dynamic Linking​: dlsrc=dl_win32.xs\, dlext=dll\, d_dlsymun=undef\, ccdlflags=' ' cccdlflags=' '\, ddlflags='-dll -nologo -nodefaultlib -release -libpath​:"d​:\perl58m\lib\CORE " -machine​:x86'

Locally applied patches​:

--- @​INC for perl v5.8.0​: D​:\Work\PerlScripts\utl D​:\Work\PerlScripts\translations D​:\Work\PerlScripts\sgml D​:\Work\PerlScripts\pleps d​:/perl58m/lib d​:/perl58m/site/lib .

--- Environment for perl v5.8.0​: HOME=d​:\apps LANG (unset) LANGUAGE (unset) LD_LIBRARY_PATH (unset) LOGDIR (unset)

PATH=d​:\perl58m\bin;d​:\sh;d​:\perl561\bin;d​:\apps\vim;d​:\apps\v slick-40\win;d :\bin;d​:\cygwin\bin;C​:\WINNT\system32;C​:\WINNT;C​:\WINNT\System 32\Wbem;d​:\bor land\cbuilder5\bin PERL5CFG=d​:\apps

PERL5LIB=D​:\Work\PerlScripts\utl;D​:\Work\PerlScripts\translati ons;D​:\Work\Pe rlScripts\sgml;D​:\Work\PerlScripts\pleps PERL_BADLANG (unset) SHELL (unset)

p5pRT commented 22 years ago

From vkonovalov@peterstar.ru

crlf.t.diff

p5pRT commented 22 years ago

From vkonovalov@peterstar.ru

perlio.c.diff

p5pRT commented 22 years ago

From nick@ing-simmons.net

Vadim Konovalov \vkonovalov@&#8203;spb\.lucent\.com writes​:

Dear all!

After some investigations I probably found a reason of misbehaviour which\, to say\, is occured not only on Win32\, but should misbehave everywhere.

Indeed that is one of the advantages of PerlIO - it behaves the same everywhere.

Patch to t/io/crlf.t introduces a test which will fail on 5.8.0\,

Good.

and patch to perlio.c should fix it.

That patch bothers me. "posn" is supposed to be the position of the start of the buffer. On to that is added ptr-buf to give actual position of the current point. That is supposed to be the case even for CRLF usage. Unconditionaly doing a ++ is wrong.

Actually "posn" struct number is "--"-ed in some code\, so I "++" it to "undo" counter mismatch.

Ah - good catch - but as the '--' is conditional on the unusual case of CR and end of prior buffer so we "fake" CRLF at start of new buffer then the "++" would need to be conditional too. However on closer inspection it is the '--' code that is wrong.

e.g. in the get_cnt code which discovers the awkward case​:

  int code;   b->ptr++; /* say we have read it as far as   * flush() is concerned */   b->buf++; /* Leave space in front of buffer */   b->bufsiz--; /* Buffer is thus smaller */   code = PerlIO_fill(f); /* Fetch some more */   b->bufsiz++; /* Restore size for next time */   b->buf--; /* Point at space */   b->ptr = nl = b->buf; /* Which is what we hand   * off */   b->posn--; /* Buffer starts here */   *nl = 0xd; /* Fill in the CR */

That code is not getting posn correct - flush does   posn = (ptr - buf) but as we have ++'ed both of those it gets posn one less than full buffer so has already yielded the posn of the CR which is what we want. So the b->posn-- is WRONG. So I think just removing the posn-- will fix it.

It might be better to experiment with removing the '--' (hence avoiding need to '++') and see if that behaves correctly. If that works it is preferable to other more convoluted patches. Comments to explain the weird side effects which cause posn to be "correct" should be added.

Nick\, could you please see if I'm correct?

There is a bug for sure. You have found the root cause. Your patch isn't right - but this is tricky stuff! I got it wrong originally and I designed it :-(

-- Nick Ing-Simmons http​://www.ni-s.u-net.com/

p5pRT commented 22 years ago

From nick@ing-simmons.net

Vadim Konovalov \vkonovalov@&#8203;peterstar\.ru writes​:

here are same patches but a bit improved.

As discussed earlier - the fix (commited as //depot/perlio/...@​17709 is as attached patch.

It passes your test (and all existing tests on Linux).

-- Nick Ing-Simmons http​://www.ni-s.u-net.com/

p5pRT commented 22 years ago

From nick@ing-simmons.net

Inline Patch ```diff --- perlio.c.ship Sun Aug 11 12:29:44 2002 +++ perlio.c Sun Aug 11 11:47:35 2002 @@ -3839,13 +3839,16 @@ b->ptr++; /* say we have read it as far as * flush() is concerned */ b->buf++; /* Leave space in front of buffer */ + /* Note as we have moved buf up flush's + posn += ptr-buf + will naturally make posn point at CR + */ b->bufsiz--; /* Buffer is thus smaller */ code = PerlIO_fill(f); /* Fetch some more */ b->bufsiz++; /* Restore size for next time */ b->buf--; /* Point at space */ b->ptr = nl = b->buf; /* Which is what we hand * off */ - b->posn--; /* Buffer starts here */ *nl = 0xd; /* Fill in the CR */ if (code == 0) goto test; /* fill() call worked */ ```
p5pRT commented 22 years ago

From vkonovalov@peterstar.ru

here are same patches but a bit improved.

As discussed earlier - the fix (commited as file​://depot/perlio/...@​17709 is as attached patch.

It passes your test (and all existing tests on Linux).

Indeed - my tests pass too. Thank you for analyzis and correct fix.

I do not insist for my test case for ./t/io/crlf.t to go in\, but will that patch go into 5.8.0+ as well? I am worrying because it was my real-life piece of code that became broken.

Best wishes\, Vadim.

p5pRT commented 21 years ago

From @jhi

The patch is part of bleadperl as of change #18086\, and maintperl (5.8.1 one day) as of change #18095. I'm marking the problem ticket as resolved.

p5pRT commented 21 years ago

@jhi - Status changed from 'new' to 'resolved'