Skip Menu |
Report information
Id: 72704
Status: resolved
Priority: 0/
Queue: perl5

Owner: Nobody
Requestors: kmxx <kmx [at] volny.cz>
Cc:
AdminCc:

Operating System: Win32
PatchStatus: (no value)
Severity: low
Type: OS-interaction
Perl Version: (no value)
Fixed In:
  • 5.12.1
  • 5.13.0



Subject: Buggy fputs(f,s) vs. fputs(s,f) on Win32 (maybe not only)
Date: Wed, 10 Feb 2010 21:43:11 +0100
To: perlbug [...] perl.org
From: kmx <kmx [...] volny.cz>
Download (untitled) / with headers
text/plain 3.9k
This is a bug report for perl from kmx@volny.cz
generated with the help of perlbug 1.39 running under perl 5.11.4.


-----------------------------------------------------------------
[Please describe your issue here]

Hi,

I have hit into a strange "maybe-bug" on Win32 platform.

The problem occurs when a XS modules tries to call function fputs() from XS/C code.

The standard UNIX declaration of fputs looks AFAIK like this:
int fputs(const char *s, FILE *stream);
 
Thus one would expect that also in XS/C code you just use "fputs(string,filehandle);" However if you do this on Win32 perl you will see an warning like this (my example comes from Math::Pari where I revealed the problem).

Pari.xs: In function 'XS_Math__Pari_dumpStack':
Pari.xs:3798: warning: passing argument 2 of '(*Perl_IStdIO_ptr((struct PerlInterpreter *)Perl_get_context()))->pPuts' from incompatible pointer type
Pari.xs:3798: note: expected 'struct FILE *' but argument is of type 'char *'
Pari.xs:3798: warning: passing argument 3 of '(*Perl_IStdIO_ptr((struct PerlInterpreter *)Perl_get_context()))->pPuts' from incompatible pointer type
Pari.xs:3798: note: expected 'const char *' but argument is of type 'struct FILE *'

Swapping the arguments fixed the warning, so it seems to me than in Win32/perl XS code it is expected to use:
fputs(filehandle,string);

whereas on Linux/perl box it is expected to use:
fputs(string,filehandle);

I have tracked this issue to iperlsys.h where I lost in a bunch of macros and typedefs.

Note: 5.11.4 used for testing is based on release tarball, gcc compiler, all options left in default.

Thanks for any help.

--
kmx


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

Configured by kmx at Wed Jan 27 16:25:21 2010.

Summary of my perl5 (revision 5 version 11 subversion 4) configuration:
   
  Platform:
    osname=MSWin32, osvers=4.0, archname=MSWin32-x86-multi-thread
    uname=''
    config_args='undef'
    hint=recommended, useposix=true, d_sigaction=undef
    useithreads=define, usemultiplicity=define
    useperlio=define, d_sfio=undef, uselargefiles=define, usesocks=undef
    use64bitint=undef, use64bitall=undef, uselongdouble=undef
    usemymalloc=n, bincompat5005=undef
  Compiler:
    cc='gcc', ccflags =' -s -O2 -DWIN32 -DHAVE_DES_FCRYPT  -DPERL_IMPLICIT_CONTEXT -DPERL_IMPLICIT_SYS -fno-strict-aliasing -mms-bitfields -DPERL_MSVCRT_READFIX',
    optimize='-s -O2',
    cppflags='-DWIN32'
    ccversion='', gccversion='4.4.3', gccosandvers=''
    intsize=4, longsize=4, ptrsize=4, doublesize=8, byteorder=1234
    d_longlong=undef, longlongsize=8, d_longdbl=define, longdblsize=12
    ivtype='long', ivsize=4, nvtype='double', nvsize=8, Off_t='long long', lseeksize=8
    alignbytes=8, prototype=define
  Linker and Libraries:
    ld='g++', ldflags ='-s -L"c:\sb\perl\lib\CORE" -L"c:\sb\c\lib"'
    libpth=c:\sb\c\lib c:\sb\c\i686-w64-mingw32\lib
    libs=-lmoldname -lkernel32 -luser32 -lgdi32 -lwinspool -lcomdlg32 -ladvapi32 -lshell32 -lole32 -loleaut32 -lnetapi32 -luuid -lws2_32 -lmpr -lwinmm -lversion -lodbc32 -lodbccp32 -lcomctl32
    perllibs=-lmoldname -lkernel32 -luser32 -lgdi32 -lwinspool -lcomdlg32 -ladvapi32 -lshell32 -lole32 -loleaut32 -lnetapi32 -luuid -lws2_32 -lmpr -lwinmm -lversion -lodbc32 -lodbccp32 -lcomctl32
    libc=, so=dll, useshrplib=true, libperl=libperl511.a
    gnulibc_version=''
  Dynamic Linking:
    dlsrc=dl_win32.xs, dlext=dll, d_dlsymun=undef, ccdlflags=' '
    cccdlflags=' ', lddlflags='-mdll -s -L"c:\sb\perl\lib\CORE" -L"c:\sb\c\lib"'

Locally applied patches:
    

---
@INC for perl 5.11.4:
    C:/sb/perl/site/lib
    C:/sb/perl/lib
    .

---
Environment for perl 5.11.4:
    HOME=D:\profile
    LANG (unset)
    LANGUAGE (unset)
    LD_LIBRARY_PATH (unset)
    LOGDIR (unset)
    PATH=C:\sb\perl\bin;C:\sb\c\bin;C:\WINDOWS\system32;C:\WINDOWS;C:\WINDOWS\System32\Wbem
    PERL_BADLANG (unset)
    PERL_MM_USE_DEFAULT=1
    SHELL (unset)

RT-Send-CC: perl5-porters [...] perl.org
Download (untitled) / with headers
text/plain 1.7k
Dne st 10.02.2010 12:44:10, kmxx napsal(a): Show quoted text
> > Hi, > > I have hit into a strange "maybe-bug" on Win32 platform. > > The problem occurs when a XS modules tries to call function fputs() > from XS/C code. > > The standard UNIX declaration of fputs looks AFAIK like this: > int fputs(const char *s, FILE *stream); > > Thus one would expect that also in XS/C code you just use > "fputs(string,filehandle);" However if you do this on Win32 perl you > will see an warning like this (my example comes from Math::Pari where > I revealed the problem). > > Pari.xs: In function 'XS_Math__Pari_dumpStack': > Pari.xs:3798: warning: passing argument 2 of '(*Perl_IStdIO_ptr > ((struct PerlInterpreter *)Perl_get_context()))->pPuts' from > incompatible pointer type > Pari.xs:3798: note: expected 'struct FILE *' but argument is of type > 'char *' > Pari.xs:3798: warning: passing argument 3 of > '(*Perl_IStdIO_ptr((struct PerlInterpreter *)Perl_get_context())) > ->pPuts' from incompatible pointer type > Pari.xs:3798: note: expected 'const char *' but argument is of type > 'struct FILE *' > > Swapping the arguments fixed the warning, so it seems to me than in > Win32/perl XS code it is expected to use: > fputs(filehandle,string); > > whereas on Linux/perl box it is expected to use: > fputs(string,filehandle); > > I have tracked this issue to iperlsys.h where I lost in a bunch of > macros and typedefs. > > Note: 5.11.4 used for testing is based on release tarball, gcc > compiler, all options left in default. > > Thanks for any help.
Can somebody please comment on this issue. I know that is not new in perl 5.11.x (perhaps not even in 5.10.x) but from my point of view it seems to be a clear bug. Thanks in advance for any suggestions. -- kmx
CC: <perl5-porters [...] perl.org>
Subject: RE: [perl #72704] Buggy fputs(f,s) vs. fputs(s,f) on Win32 (maybe not only)
Date: Thu, 4 Mar 2010 16:40:41 -0000
To: <perlbug-followup [...] perl.org>, "OtherRecipients of perl Ticket #72704" <"OtherRecipients of perl Ticket #72704:;" [...] planit.com>
From: "Steve Hay" <SteveHay [...] planit.com>
Download (untitled) / with headers
text/plain 2.2k
kmx via RT wrote on 2010-03-03: Show quoted text
> Dne st 10.02.2010 12:44:10, kmxx napsal(a):
>> >> Hi, >> >> I have hit into a strange "maybe-bug" on Win32 platform. >> >> The problem occurs when a XS modules tries to call function fputs() >> from XS/C code. >> >> The standard UNIX declaration of fputs looks AFAIK like this: >> int fputs(const char *s, FILE *stream); >> >> Thus one would expect that also in XS/C code you just use >> "fputs(string,filehandle);" However if you do this on Win32 perl you >> will see an warning like this (my example comes from Math::Pari where >> I revealed the problem). >> >> Pari.xs: In function 'XS_Math__Pari_dumpStack': >> Pari.xs:3798: warning: passing argument 2 of '(*Perl_IStdIO_ptr >> ((struct PerlInterpreter *)Perl_get_context()))->pPuts' from >> incompatible pointer type >> Pari.xs:3798: note: expected 'struct FILE *' but argument is of type >> 'char *' >> Pari.xs:3798: warning: passing argument 3 of >> '(*Perl_IStdIO_ptr((struct PerlInterpreter *)Perl_get_context())) >> ->pPuts' from incompatible pointer type >> Pari.xs:3798: note: expected 'const char *' but argument is of type >> 'struct FILE *' >> >> Swapping the arguments fixed the warning, so it seems to me than in >> Win32/perl XS code it is expected to use: >> fputs(filehandle,string); >> >> whereas on Linux/perl box it is expected to use: >> fputs(string,filehandle); >> >> I have tracked this issue to iperlsys.h where I lost in a bunch of >> macros and typedefs. >> >> Note: 5.11.4 used for testing is based on release tarball, gcc >> compiler, all options left in default. >> >> Thanks for any help.
> > Can somebody please comment on this issue. I know that is not new in > perl 5.11.x (perhaps not even in 5.10.x) but from my point of view it > seems to be a clear bug. > > Thanks in advance for any suggestions. >
Sorry, I meant to reply to this earlier but completely forgot. I think it is deliberate: it is explicitly mentioned in perlapio.pod that the arguments have been reversed compared to the C library function (probably so that all the PerlIO functions have the 'f' first). And there's a further comment about it in perlxstut.pod too. Are you sure the arguments are not reversed on Linux too? There's no mention of the reversal being Win32-specific.
CC: perlbug-followup [...] perl.org, OtherRecipients of perl Ticket #72704 <"OtherRecipients of perl Ticket #72704:;" [...] planit.com>, perl5-porters [...] perl.org
Subject: Re: [perl #72704] Buggy fputs(f,s) vs. fputs(s,f) on Win32 (maybe not only)
Date: Thu, 4 Mar 2010 20:17:59 +0000
To: Steve Hay <SteveHay [...] planit.com>
From: Dave Mitchell <davem [...] iabyn.com>
Download (untitled) / with headers
text/plain 1.5k
On Thu, Mar 04, 2010 at 04:40:41PM -0000, Steve Hay wrote: Show quoted text
> Sorry, I meant to reply to this earlier but completely forgot. > > I think it is deliberate: it is explicitly mentioned in perlapio.pod that the arguments have been reversed compared to the C library function (probably so that all the PerlIO functions have the 'f' first). And there's a further comment about it in perlxstut.pod too. > > Are you sure the arguments are not reversed on Linux too? There's no mention of the reversal being Win32-specific.
While the args of the PerlIO functions are explicity reversed, I think that OP is right, in that something is probably awry with the macro expansions. For example in fakesdio.h, you have #define fputs(s,f) PerlIO_puts(f,s) which does the correct arg reversal. *However*: out in XS/win32 land, you appear to have: XSUB.h: #define fputs PerlSIO_fputs iperlsys.h: #if defined(PERL_IMPLICIT_SYS) #define PerlSIO_fputs(f,s) (*PL_StdIO->pPuts)(PL_StdIO, (f),(s)) #else #define PerlSIO_fputs(f,s) fputs(s,f) #endif and the type of function expected in the pPuts slot is LPPuts, which is defined as typedef int (*LPPuts)(struct IPerlStdIO*, FILE*, const char*); So under some circumstances, fputs(str,handle) => PerlIO_puts(str,handle) => (*PL_StdIO->pPuts)(PL_StdIO, (str),(handle)) which doesn't match the LPPuts definition. But that's a horrible twisty maze of macros that I don't really understand. -- Any [programming] language that doesn't occasionally surprise the novice will pay for it by continually surprising the expert. -- Larry Wall
CC: Steve Hay <SteveHay [...] planit.com>, perlbug-followup [...] perl.org, "OtherRecipients of perl Ticket #72704" <"OtherRecipients of perl Ticket #72704:," [...] planit.com>, perl5-porters [...] perl.org, Jan Dubois <jand [...] activestate.com>
Subject: Re: [perl #72704] Buggy fputs(f,s) vs. fputs(s,f) on Win32 (maybe not only)
Date: Fri, 5 Mar 2010 00:17:31 +0000
To: Dave Mitchell <davem [...] iabyn.com>
From: Steve Hay <steve.m.hay [...] googlemail.com>
Download (untitled) / with headers
text/plain 4.1k
On 4 March 2010 20:17, Dave Mitchell <davem@iabyn.com> wrote: Show quoted text
> On Thu, Mar 04, 2010 at 04:40:41PM -0000, Steve Hay wrote:
>> Sorry, I meant to reply to this earlier but completely forgot. >> >> I think it is deliberate: it is explicitly mentioned in perlapio.pod that the arguments have been reversed compared to the C library function (probably so that all the PerlIO functions have the 'f' first). And there's a further comment about it in perlxstut.pod too. >> >> Are you sure the arguments are not reversed on Linux too? There's no mention of the reversal being Win32-specific.
> > > While the args of the PerlIO functions are explicity reversed, I think > that OP is right, in that something is probably awry with the macro > expansions. > > > For example in fakesdio.h, you have > >    #define fputs(s,f) PerlIO_puts(f,s) > > which does the correct arg reversal. > > *However*: out in XS/win32 land, you appear to have: > > XSUB.h: #define fputs PerlSIO_fputs > > iperlsys.h: > >    #if defined(PERL_IMPLICIT_SYS) >        #define PerlSIO_fputs(f,s) (*PL_StdIO->pPuts)(PL_StdIO, (f),(s)) >    #else >        #define PerlSIO_fputs(f,s)              fputs(s,f) >    #endif > > and the type of function expected in the pPuts slot is LPPuts, which is > defined as > >    typedef int         (*LPPuts)(struct IPerlStdIO*, FILE*, const char*); > > > So under some circumstances, > >    fputs(str,handle) > =>  PerlIO_puts(str,handle) > =>  (*PL_StdIO->pPuts)(PL_StdIO, (str),(handle)) > > which doesn't match the LPPuts definition. >
I'm not claiming to fully understand this macro maze myself, but I also don't follow how you arrived at your three lines above. The way I see it (which could be entirely wrong...) is this: If you're using PerlIO and you want to access an original CRT IO function then you should use one of the PerlLIO_* or PerlSIO_* names which are specifically designed for this purpose. iperlsys.h defines PerlSIO_fputs(f, s) as (*PL_StdIO->pPuts)(PL_StdIO, (f),(s)) when using PERL_IMPLICIT_SYS (with the pPuts slot typedef'ed as you show above: with the FILE* before the const char*), or fputs(s, f) otherwise. The latter is the original CRT function, with its args put back to their original order. The former goes through the host layer code which allows embedders to redefine functions where necessary. The default implementation on Win32 is win32/perlhost.h, in which the pPuts slot is implemented as int PerlStdIOPuts(struct IPerlStdIO* piPerl, FILE* pf, const char *s) { return win32_fputs(s, pf); } (note that the args get reversed in there!) and in win32/win32.c we have DllExport int win32_fputs(const char *s,FILE *pf) { return fputs(s, pf); } i.e. the default implementation simply takes us back to the original CRT function again. There's also something in win32/win32iop.h which I'm not entirely clear about: it additionally redirects fputs to win32_fputs if WIN32IO_IS_STDIO is not defined, which is only the case when PERL_IMPLICIT_SYS is not defined (see win32/win32.h). I think maybe that is so that XS authors can lazily write fputs() instead of the more correct PerlSIO_fputs(), but it only works if you don't have PERL_IMPLICIT_SYS defined. (If this is right, I'm not sure why the shortcut isn't provided for PERL_IMPLICIT_SYS. Possibly it wouldn't make sense somehow, but we'd need someone who actually understands the host layer stuff to explain. Jan?) The OP is using Strawberry Perl, which does build with PERL_IMPLICIT_SYS defined, and hence doesn't get that lazy shortcut. So the problem could simply be that one *needs* to use the PerlLIO / PerlSIO names under PERL_IMPLICIT_SYS and perhaps the OP didn't do so (no doubt for the reason following below...)? The other big problem here is that none of the PerlLIO / PerlSIO functions or their use appears to be documented anywhere. I learned of them from the (sorely missed) Nick I-S when writing Win32-SharedFileOpen (which proved more useful as a learning exercise than as anything else). The Changes file notes that exactly such a problem was fixed in 3.10, and SharedFileOpen.xs contains a large comment about it all near the top. I guess the problem doesn't bite often because only Win32 makes use of PERL_IMPLICIT_SYS (?).
CC: <perlbug-followup [...] perl.org>, <perl5-porters [...] perl.org>
Subject: RE: [perl #72704] Buggy fputs(f,s) vs. fputs(s,f) on Win32 (maybe not only)
Date: Thu, 4 Mar 2010 18:53:29 -0800
To: "'Dave Mitchell'" <davem [...] iabyn.com>, "'Steve Hay'" <SteveHay [...] planit.com>
From: "Jan Dubois" <jand [...] activestate.com>
Download (untitled) / with headers
text/plain 3.4k
On Thu, 04 Mar 2010, Dave Mitchell wrote: Show quoted text
> *However*: out in XS/win32 land, you appear to have: > > XSUB.h: #define fputs PerlSIO_fputs
That one is correct. PerlSIO_* APIs are supposed to have the same prototype as their corresponding stdio ones. Show quoted text
> iperlsys.h: > > #if defined(PERL_IMPLICIT_SYS) > #define PerlSIO_fputs(f,s) (*PL_StdIO->pPuts)(PL_StdIO, (f),(s)) > #else > #define PerlSIO_fputs(f,s) fputs(s,f) > #endif
This however is busted. The parameters should not be swapped. The same problem exists for the fputc() function as well. I've attached my attempt at correcting all the mistakes I found. Core Perl still builds fine on Windows and OS X; but I haven't tried building any extensions that use fputs() or fputc(). In case the patch gets mangled by email and/or RT, you can read it on github: http://github.com/jandubois/perl/commit/8656122df48f238e6526299295e8f96524ac4158 or download the raw diff at: http://github.com/jandubois/perl/commit/8656122df48f238e6526299295e8f96524ac4158.diff So please test and report back if this fixes the problem for Math::Pari on Windows! Cheers, -Jan --- a/iperlsys.h +++ b/iperlsys.h @@ -78,8 +78,8 @@ typedef int (*LPGetCnt)(struct IPerlStdIO*, FILE*); typedef STDCHAR* (*LPGetPtr)(struct IPerlStdIO*, FILE*); typedef char* (*LPGets)(struct IPerlStdIO*, FILE*, char*, int); -typedef int (*LPPutc)(struct IPerlStdIO*, FILE*, int); -typedef int (*LPPuts)(struct IPerlStdIO*, FILE*, const char*); +typedef int (*LPPutc)(struct IPerlStdIO*, int, FILE*); +typedef int (*LPPuts)(struct IPerlStdIO*, const char *, FILE*); typedef int (*LPFlush)(struct IPerlStdIO*, FILE*); typedef int (*LPUngetc)(struct IPerlStdIO*, int,FILE*); typedef int (*LPFileno)(struct IPerlStdIO*, FILE*); @@ -225,10 +225,10 @@ (*PL_StdIO->pGetCnt)(PL_StdIO, (f)) #define PerlSIO_get_ptr(f) \ (*PL_StdIO->pGetPtr)(PL_StdIO, (f)) -#define PerlSIO_fputc(f,c) \ - (*PL_StdIO->pPutc)(PL_StdIO, (f),(c)) -#define PerlSIO_fputs(f,s) \ - (*PL_StdIO->pPuts)(PL_StdIO, (f),(s)) +#define PerlSIO_fputc(c,f) \ + (*PL_StdIO->pPutc)(PL_StdIO, (c),(f)) +#define PerlSIO_fputs(s,f) \ + (*PL_StdIO->pPuts)(PL_StdIO, (s),(f)) #define PerlSIO_fflush(f) \ (*PL_StdIO->pFlush)(PL_StdIO, (f)) #define PerlSIO_fgets(s, n, fp) \ @@ -311,8 +311,8 @@ #define PerlSIO_get_cnt(f) 0 #define PerlSIO_get_ptr(f) NULL #endif -#define PerlSIO_fputc(f,c) fputc(c,f) -#define PerlSIO_fputs(f,s) fputs(s,f) +#define PerlSIO_fputc(c,f) fputc(c,f) +#define PerlSIO_fputs(s,f) fputs(s,f) #define PerlSIO_fflush(f) Fflush(f) #define PerlSIO_fgets(s, n, fp) fgets(s,n,fp) #if defined(VMS) && defined(__DECC) --- a/perlsdio.h +++ b/perlsdio.h @@ -34,8 +34,8 @@ #define PerlIO_fdopen PerlSIO_fdopen #define PerlIO_reopen PerlSIO_freopen #define PerlIO_close(f) PerlSIO_fclose(f) -#define PerlIO_puts(f,s) PerlSIO_fputs(f,s) -#define PerlIO_putc(f,c) PerlSIO_fputc(f,c) +#define PerlIO_puts(f,s) PerlSIO_fputs(s,f) +#define PerlIO_putc(f,c) PerlSIO_fputc(c,f) #if defined(VMS) # if defined(__DECC) /* Unusual definition of ungetc() here to accomodate fast_sv_gets()' --- a/win32/perlhost.h +++ b/win32/perlhost.h @@ -669,13 +669,13 @@ } int -PerlStdIOPutc(struct IPerlStdIO* piPerl, FILE* pf, int c) +PerlStdIOPutc(struct IPerlStdIO* piPerl, int c, FILE* pf) { return win32_fputc(c, pf); } int -PerlStdIOPuts(struct IPerlStdIO* piPerl, FILE* pf, const char *s) +PerlStdIOPuts(struct IPerlStdIO* piPerl, const char *s, FILE* pf) { return win32_fputs(s, pf); }
CC: <perlbug-followup [...] perl.org>, <perl5-porters [...] perl.org>
Subject: RE: [perl #72704] Buggy fputs(f,s) vs. fputs(s,f) on Win32 (maybe not only)
Date: Thu, 4 Mar 2010 19:08:22 -0800
To: "'Dave Mitchell'" <davem [...] iabyn.com>, "'Steve Hay'" <SteveHay [...] planit.com>
From: "Jan Dubois" <jand [...] activestate.com>
Download (untitled) / with headers
text/plain 1.4k
On Thu, 04 Mar 2010, Jan Dubois wrote: Show quoted text
> On Thu, 04 Mar 2010, Dave Mitchell wrote:
> > *However*: out in XS/win32 land, you appear to have: > > > > XSUB.h: #define fputs PerlSIO_fputs
> > That one is correct. PerlSIO_* APIs are supposed to have the same prototype > as their corresponding stdio ones. >
> > iperlsys.h: > > > > #if defined(PERL_IMPLICIT_SYS) > > #define PerlSIO_fputs(f,s) (*PL_StdIO->pPuts)(PL_StdIO, (f),(s)) > > #else > > #define PerlSIO_fputs(f,s) fputs(s,f) > > #endif
> > This however is busted. The parameters should not be swapped. > > The same problem exists for the fputc() function as well. I've attached my > attempt at correcting all the mistakes I found. Core Perl still builds > fine on Windows and OS X; but I haven't tried building any extensions that > use fputs() or fputc().
Note that is would be _possible_ to make a much smaller patch than the one I supplied by just swapping the parameters in #define PerlSIO_fputs(f,s) (*PL_StdIO->pPuts)(PL_StdIO, (f),(s)) (and also in the corresponding PerlSIO_fputc() #define). It would however be rather confusing longer term, as that works only accidentally: perlsdio.h doesn't swap the parameter order when it should, and iperlsys.h then compensates for that by swapping when it shouldn't. So I would much prefer to use the comprehensive patch provided in my previous message to reduce the amount of brain-pain when somebody looks at these again in the future. Cheers, -Jan
Download (untitled) / with headers
text/plain 463b
Hi, firts of all thaks for your effort you have invested into this issue. Show quoted text
> So please test and report back if this fixes the problem for > Math::Pari on Windows!
I will do, however my build env is slighly disorganised at the moment so it will take approx. a week. But I have prepared a small failing test example - see attached TestFputs-0.01.tar.gz (just do perl Makefile.Pl + make test). If anybody is able to test your patch earlier. Thanks again. -- kmx
Download TestFputs-0.01.tar.gz
application/gzip 13.1k

Message body not shown because it is not plain text.

RT-Send-CC: perl5-porters [...] perl.org
Download (untitled) / with headers
text/plain 667b
I forgot to Cc: p5p list Dne čt 04.bře.2010 22:09:09, kmxx napsal(a): Show quoted text
> Hi, > > firts of all thaks for your effort you have invested into this issue. >
> > So please test and report back if this fixes the problem for > > Math::Pari on Windows!
> > I will do, however my build env is slighly disorganised at the > moment so it will take approx. a week. > > But I have prepared a small failing test example - see attached > TestFputs-0.01.tar.gz (just do perl Makefile.Pl + make test). If > anybody is able to test your patch earlier. > > Thanks again. > > -- > kmx
attachment link: http://rt.perl.org/rt3/Ticket/Attachment/664560/317278/TestFputs-0.01.tar.gz
CC: "'Steve Hay'" <SteveHay [...] planit.com>, <perlbug-followup [...] perl.org>, <perl5-porters [...] perl.org>
Subject: RE: [perl #72704] Buggy fputs(f,s) vs. fputs(s,f) on Win32 (maybe not only)
Date: Fri, 5 Mar 2010 00:27:03 -0800
To: "'Steve Hay'" <steve.m.hay [...] googlemail.com>, "'Dave Mitchell'" <davem [...] iabyn.com>
From: "Jan Dubois" <jand [...] activestate.com>
Download (untitled) / with headers
text/plain 2.5k
On Thu, 04 Mar 2010, Steve Hay wrote: Show quoted text
> The way I see it (which could be entirely wrong...) is this: > > If you're using PerlIO and you want to access an original CRT IO > function then you should use one of the PerlLIO_* or PerlSIO_* names > which are specifically designed for this purpose.
I believe that is wrong. At least the PerlSIO_* names are an implementation detail and should not be used directly from XS extensions at all. XS code should not have to care if it is being compiled using PerlIO or not, and it should not matter if PERL_IMPLICIT_SYS is being used either. The code should be able to use the standard CRT functions, and the Perl macros will redefine them to do the right thing for the actual set of Perl build options in effect. Show quoted text
> There's also something in win32/win32iop.h which I'm not entirely > clear about: it additionally redirects fputs to win32_fputs if > WIN32IO_IS_STDIO is not defined, which is only the case when > PERL_IMPLICIT_SYS is not defined (see win32/win32.h). I think maybe > that is so that XS authors can lazily write fputs() instead of the > more correct PerlSIO_fputs(), but it only works if you don't have > PERL_IMPLICIT_SYS defined. (If this is right, I'm not sure why the > shortcut isn't provided for PERL_IMPLICIT_SYS. Possibly it wouldn't > make sense somehow, but we'd need someone who actually understands the > host layer stuff to explain. Jan?)
XSUB.h already redefines fputs to PerlSIO_fputs for PERL_IMPLICIT_SYS. Without PERL_IMPLICIT_SYS PerlSIO_fputs would eventually be redefined back to fputs to call the C RTL version. The win32/win32iop.h defines make sure it instead ends up at win32_fputs(). This doesn't matter for fputs(), but for any function that we have "fixed" (e.g. stat()) we want the XS code to call our implementation. Note that this is just a theory; I have not verified that it actually works. I never build Perl on Windows without PERL_IMPLICIT_SYS, and I doubt many other people do. Show quoted text
> The other big problem here is that none of the PerlLIO / PerlSIO > functions or their use appears to be documented anywhere. I learned of > them from the (sorely missed) Nick I-S when writing > Win32-SharedFileOpen (which proved more useful as a learning exercise > than as anything else). The Changes file notes that exactly such a > problem was fixed in 3.10, and SharedFileOpen.xs contains a large > comment about it all near the top.
I don't understand why you need to call PerlSIO_flcose() in that module given that XSUB.h already redefines fclose() to PerlSIO_fclose under PERL_IMPLICIT_SYS. Cheers, -Jan
CC: <perlbug-followup [...] perl.org>, <perl5-porters [...] perl.org>
Subject: RE: [perl #72704] Buggy fputs(f,s) vs. fputs(s,f) on Win32 (maybe not only)
Date: Fri, 5 Mar 2010 09:19:14 -0000
To: "Jan Dubois" <jand [...] activestate.com>, "Steve Hay" <steve.m.hay [...] googlemail.com>, "Dave Mitchell" <davem [...] iabyn.com>
From: "Steve Hay" <SteveHay [...] planit.com>
Download (untitled) / with headers
text/plain 3.9k
Jan Dubois wrote on 2010-03-05: Show quoted text
> On Thu, 04 Mar 2010, Steve Hay wrote:
>> The way I see it (which could be entirely wrong...) is this: >> >> If you're using PerlIO and you want to access an original CRT IO >> function then you should use one of the PerlLIO_* or PerlSIO_* names >> which are specifically designed for this purpose.
> I believe that is wrong. At least the PerlSIO_* names are an > implementation detail and should not be used directly from XS extensions > at all.
Clearly I'm still on a learning exercise now ;-) Show quoted text
> > XS code should not have to care if it is being compiled using PerlIO > or not, and it should not matter if PERL_IMPLICIT_SYS is being used > either. The code should be able to use the standard CRT functions, > and the Perl macros will redefine them to do the right thing for > the actual set of Perl build options in effect.
Sounds good, but it didn't work at the time. It does now, though, after trying it again with a current perl... Show quoted text
>
>> There's also something in win32/win32iop.h which I'm not entirely clear >> about: it additionally redirects fputs to win32_fputs if >> WIN32IO_IS_STDIO is not defined, which is only the case when >> PERL_IMPLICIT_SYS is not defined (see win32/win32.h). I think maybe >> that is so that XS authors can lazily write fputs() instead of the more >> correct PerlSIO_fputs(), but it only works if you don't have >> PERL_IMPLICIT_SYS defined. (If this is right, I'm not sure why the >> shortcut isn't provided for PERL_IMPLICIT_SYS. Possibly it wouldn't >> make sense somehow, but we'd need someone who actually understands the >> host layer stuff to explain. Jan?)
> > XSUB.h already redefines fputs to PerlSIO_fputs for PERL_IMPLICIT_SYS. > Without PERL_IMPLICIT_SYS PerlSIO_fputs would eventually be redefined > back to fputs to call the C RTL version. The win32/win32iop.h > defines make sure it instead ends up at win32_fputs(). This doesn't > matter for fputs(), but for any function that we have "fixed" (e.g. > stat()) we want the XS code to call our implementation. > > Note that this is just a theory; I have not verified that it actually > works. I never build Perl on Windows without PERL_IMPLICIT_SYS, and > I doubt many other people do. >
>> The other big problem here is that none of the PerlLIO / PerlSIO >> functions or their use appears to be documented anywhere. I learned of >> them from the (sorely missed) Nick I-S when writing >> Win32-SharedFileOpen (which proved more useful as a learning exercise >> than as anything else). The Changes file notes that exactly such a >> problem was fixed in 3.10, and SharedFileOpen.xs contains a large >> comment about it all near the top.
> I don't understand why you need to call PerlSIO_flcose() in that module > given that XSUB.h already redefines fclose() to PerlSIO_fclose under > PERL_IMPLICIT_SYS. >
I just looked at the module again. The Changes file says fclose() was changed to PerlSIO_fclose() to allow building with perl-5.8.0 with PERL_IMPLICIT_SYS, so I tried that with the PerlSIO_ prefix deleted. It builds and passes all tests, but emits two compiler warnings: SharedFileOpen.xs(499) : warning C4047: 'function' : 'struct _PerlIO ** ' differs in levels of indirection from 'struct _iobuf *' SharedFileOpen.xs(499) : warning C4024: 'Perl_PerlIO_close' : different types for formal and actual parameter 2 Using PerlSIO_fclose() instead fixes that. However, trying with bleadperl I don't get the same result. The problem is that perl-5.8.0's XSUB.h didn't redefine fclose to PerlSIO_fclose: it redefined it to PerlIO_close instead. It got fixed (by NI-S) on 26 Jan 2003, the very day after I uploaded my "fixed" CPAN module: http://perl5.git.perl.org/perl.git/commit/f9415d2 So it looks like I should undo all my PerlSIO_/PerlLIO_ stuff, and add a local fix for the fclose bug in perl-5.8.0 to maintain compatibility with that version of perl. You live and learn.
CC: <perl5-porters [...] perl.org>
Subject: RE: [perl #72704] Buggy fputs(f,s) vs. fputs(s,f) on Win32 (maybe not only)
Date: Fri, 5 Mar 2010 12:50:34 -0800
To: <perlbug-followup [...] perl.org>
From: "Jan Dubois" <jand [...] activestate.com>
Download (untitled) / with headers
text/plain 1.9k
Show quoted text
> Dne čt 04.bře.2010 22:09:09, kmxx napsal(a):
> > Hi, > > > > firts of all thaks for your effort you have invested into this > > issue. > >
> > > So please test and report back if this fixes the problem for > > > Math::Pari on Windows!
> > > > I will do, however my build env is slighly disorganised at the > > moment so it will take approx. a week. > > > > But I have prepared a small failing test example - see attached > > TestFputs-0.01.tar.gz (just do perl Makefile.Pl + make test). If > > anybody is able to test your patch earlier.
Yes, the patch fixes the issue exhibited by your test example. Given the current code freeze this fix will have to wait until after the 5.12 release, unless someone wants to convince Jesse that this should be a blocker bug. I could see it either way, but given that this bug has been in Perl since at least 5.8.0 it is hard to argue that it should be *blocking* a Perl release. So assuming it won't make it into 5.12 I plan to add the attached simple fix to ActivePerl, and I would suggest Strawberry does the same. It does not sanitize the macros to be internally consistent, but it fixes the bug in a backward compatible manner (as long as XS code doesn't call PerlSIO_fputs() directly). Cheers, -Jan --- iperlsys.h~ 2010-02-09 12:34:54.000000000 -0800 +++ iperlsys.h 2010-03-05 12:24:17.674828900 -0800 @@ -226,9 +226,9 @@ #define PerlSIO_get_ptr(f) \ (*PL_StdIO->pGetPtr)(PL_StdIO, (f)) #define PerlSIO_fputc(f,c) \ - (*PL_StdIO->pPutc)(PL_StdIO, (f),(c)) + (*PL_StdIO->pPutc)(PL_StdIO, (c),(f)) #define PerlSIO_fputs(f,s) \ - (*PL_StdIO->pPuts)(PL_StdIO, (f),(s)) + (*PL_StdIO->pPuts)(PL_StdIO, (s),(f)) #define PerlSIO_fflush(f) \ (*PL_StdIO->pFlush)(PL_StdIO, (f)) #define PerlSIO_fgets(s, n, fp) \
Download (untitled) / with headers
text/plain 1.5k
On Fri Mar 05 12:51:32 2010, jdb wrote: Show quoted text
> > Dne čt 04.bře.2010 22:09:09, kmxx napsal(a):
> > > Hi, > > > > > > firts of all thaks for your effort you have invested into this > > > issue. > > >
> > > > So please test and report back if this fixes the problem for > > > > Math::Pari on Windows!
> > > > > > I will do, however my build env is slighly disorganised at the > > > moment so it will take approx. a week. > > > > > > But I have prepared a small failing test example - see attached > > > TestFputs-0.01.tar.gz (just do perl Makefile.Pl + make test). If > > > anybody is able to test your patch earlier.
> > Yes, the patch fixes the issue exhibited by your test example. > > Given the current code freeze this fix will have to wait until after > the 5.12 release, unless someone wants to convince Jesse that this should > be a blocker bug. I could see it either way, but given that this bug > has been in Perl since at least 5.8.0 it is hard to argue that it should > be *blocking* a Perl release. > > So assuming it won't make it into 5.12 I plan to add the attached simple > fix to ActivePerl, and I would suggest Strawberry does the same. It does > not sanitize the macros to be internally consistent, but it fixes the > bug in a backward compatible manner (as long as XS code doesn't call > PerlSIO_fputs() directly).
I have one question. Would this patch be required for 5.10.x versions of Perl, as well, or just for 5.11.x/5.12.x? [The reason I ask is that I plan to build a 5.10.x version of strawberry until 5.12.1.1 is out.] --Curtis
CC: perlbug-followup [...] perl.org, perl5-porters [...] perl.org
Subject: Re: [perl #72704] Buggy fputs(f,s) vs. fputs(s,f) on Win32 (maybe not only)
Date: Sat, 6 Mar 2010 21:38:10 +0000
To: Jan Dubois <jand [...] activestate.com>
From: Tim Bunce <Tim.Bunce [...] pobox.com>
Download (untitled) / with headers
text/plain 2.2k
Could this be the cause of this NYTProf failure on Windows? https://rt.cpan.org/Ticket/Display.html?id=55049 Tim. On Fri, Mar 05, 2010 at 12:50:34PM -0800, Jan Dubois wrote: Show quoted text
> > Dne čt 04.bře.2010 22:09:09, kmxx napsal(a):
> > > Hi, > > > > > > firts of all thaks for your effort you have invested into this > > > issue. > > >
> > > > So please test and report back if this fixes the problem for > > > > Math::Pari on Windows!
> > > > > > I will do, however my build env is slighly disorganised at the > > > moment so it will take approx. a week. > > > > > > But I have prepared a small failing test example - see attached > > > TestFputs-0.01.tar.gz (just do perl Makefile.Pl + make test). If > > > anybody is able to test your patch earlier.
> > Yes, the patch fixes the issue exhibited by your test example. > > Given the current code freeze this fix will have to wait until after > the 5.12 release, unless someone wants to convince Jesse that this should > be a blocker bug. I could see it either way, but given that this bug > has been in Perl since at least 5.8.0 it is hard to argue that it should > be *blocking* a Perl release. > > So assuming it won't make it into 5.12 I plan to add the attached simple > fix to ActivePerl, and I would suggest Strawberry does the same. It does > not sanitize the macros to be internally consistent, but it fixes the > bug in a backward compatible manner (as long as XS code doesn't call > PerlSIO_fputs() directly). > > Cheers, > -Jan > > --- iperlsys.h~ 2010-02-09 12:34:54.000000000 -0800 > +++ iperlsys.h 2010-03-05 12:24:17.674828900 -0800 > @@ -226,9 +226,9 @@ > #define PerlSIO_get_ptr(f) \ > (*PL_StdIO->pGetPtr)(PL_StdIO, (f)) > #define PerlSIO_fputc(f,c) \ > - (*PL_StdIO->pPutc)(PL_StdIO, (f),(c)) > + (*PL_StdIO->pPutc)(PL_StdIO, (c),(f)) > #define PerlSIO_fputs(f,s) \ > - (*PL_StdIO->pPuts)(PL_StdIO, (f),(s)) > + (*PL_StdIO->pPuts)(PL_StdIO, (s),(f)) > #define PerlSIO_fflush(f) \ > (*PL_StdIO->pFlush)(PL_StdIO, (f)) > #define PerlSIO_fgets(s, n, fp) \ > >
Subject: Re: [perl #72704] Buggy fputs(f,s) vs. fputs(s,f) on Win32 (maybe not only)
Date: Sat, 06 Mar 2010 08:52:25 +0100
To: perlbug-followup [...] perl.org
From: kmx <kmx [...] volny.cz>
Download (untitled) / with headers
text/plain 1.2k
Show quoted text
>> Yes, the patch fixes the issue exhibited by your test example. >>
Thanks for testing, Jan. Show quoted text
>> Given the current code freeze this fix will have to wait until after >> the 5.12 release, unless someone wants to convince Jesse that this should >> be a blocker bug. I could see it either way, but given that this bug >> has been in Perl since at least 5.8.0 it is hard to argue that it should >> be *blocking* a Perl release. >> >> So assuming it won't make it into 5.12 I plan to add the attached simple >> fix to ActivePerl, and I would suggest Strawberry does the same. It does >> not sanitize the macros to be internally consistent, but it fixes the >> bug in a backward compatible manner (as long as XS code doesn't call >> PerlSIO_fputs() directly). >>
>
From that point of view it is not a blocker as both ActivePerl + Strawberry (and other Win32 perls) live with it for years. However it is definitely long lasting defect that hurts mostly (if not only) Win32 perl users. If not accepted in 5.12 please do not forget to commit this patch after code unfreeze. Show quoted text
> I have one question. Would this patch be required for 5.10.x versions of > Perl, as well, or just for 5.11.x/5.12.x? >
Definitely yes. -- kmx
RT-Send-CC: perl5-porters [...] perl.org
May I nominate the patch fixings RT proposed by Jan to get into 5.12.1? Thanks. -- kmx
Download (untitled) / with headers
text/plain 605b
Dne čt 04.bře.2010 18:55:26, jdb napsal(a): Show quoted text
> > ... > > The same problem exists for the fputc() function as well. I've > attached my attempt at correcting all the mistakes I found. Core > Perl still builds fine on Windows and OS X; but I haven't tried > building any extensions that use fputs() or fputc(). > > In case the patch gets mangled by email and/or RT, you can read > it on github: > > http://github.com/jandubois/perl/commit/8656122df48f
Now - after blead unfreeze - can I ask somebody (perhaps Jan, the author of the above mentioned patch) to commit this fix to blead? Thanks. -- kmx
RT-Send-CC: perl5-porters [...] perl.org
Download (untitled) / with headers
text/plain 460b
On Wed Apr 14 23:55:56 2010, kmxx wrote: Show quoted text
> Now - after blead unfreeze - can I ask somebody (perhaps Jan, the author > of the above mentioned patch) to commit this fix to blead?
Fix has been committed as change 634b482c. I'll ask for supporting votes to integrate into maint-5.12 once rgs has his new tool up and running. Same thing for commit 20c8f8f which is similar (fgets() doesn't have swapped parameters, it is missing a redefinition macro completely).
RT-Send-CC: perl5-porters [...] perl.org
Download (untitled) / with headers
text/plain 440b
Dne st 21.dub.2010 23:55:55, jdb napsal(a): Show quoted text
> ... > > Fix has been committed as change 634b482c. > > I'll ask for supporting votes to integrate into maint-5.12 once rgs has > his new tool up and running. > > Same thing for commit 20c8f8f which is similar (fgets() doesn't have > swapped parameters, it is missing a redefinition macro completely).
If my vote counts I would like to support including 634b482c + 20c8f8f to 5.12.1 -- kmx
RT-Send-CC: perl5-porters [...] perl.org
Download (untitled) / with headers
text/plain 124b
I see comming deadline for 5.12.1. Could somebody competent consider including 634b482c + 20c8f8f to 5.12.1 Thanks -- kmx
CC: <perl5-porters [...] perl.org>
Subject: RE: [perl #72704] Buggy fputs(f,s) vs. fputs(s,f) on Win32 (maybe not only)
Date: Wed, 5 May 2010 12:27:02 -0700
To: <perlbug-followup [...] perl.org>, <tim.bunce [...] pobox.com>
From: "Jan Dubois" <jand [...] activestate.com>
Download (untitled) / with headers
text/plain 1.4k
On Wed, 05 May 2010, kmx via RT wrote: Show quoted text
> I see comming deadline for 5.12.1. > > Could somebody competent consider including 634b482c + 20c8f8f > to 5.12.1
634b482c is already in maint-5.12 as 72c7417: http://perl5.git.perl.org/perl.git/commitdiff/72c7417 20c8f8f has been requested (by me) and got a second vote from Vincent. It requires one other person with commit access to vote for it to be eligible for merging. Given that this bug was detected with Devel::NYTProf, maybe Tim is willing to put his vote in for this one (it is a single line addition to XSUB.h). Cheers, -Jan commit 20c8f8f9118fd23081c818637815bf1aab60b808 Author: Jan Dubois <jand@activestate.com> Date: Wed Apr 21 16:49:09 2010 -0700 XSUB.h is supposed to redefine fgets under PERL_IMPLICIT_SYS, but doesn't. See also http://rt.cpan.org/Public/Bug/Display.html?id=55049 with workaround in http://code.google.com/p/perl-devel-nytprof/source/detail?r=1168 diff --git a/XSUB.h b/XSUB.h index f23df37..06cb1c3 100644 --- a/XSUB.h +++ b/XSUB.h @@ -507,6 +507,7 @@ Rethrows a previously caught exception. See L<perlguts/"Exception # define ferror PerlSIO_ferror # define clearerr PerlSIO_clearerr # define getc PerlSIO_getc +# define fgets PerlSIO_fgets # define fputc PerlSIO_fputc # define fputs PerlSIO_fputs # define fflush PerlSIO_fflush
CC: perlbug-followup [...] perl.org, tim.bunce [...] pobox.com, perl5-porters [...] perl.org
Subject: Re: [perl #72704] Buggy fputs(f,s) vs. fputs(s,f) on Win32 (maybe not only)
Date: Thu, 6 May 2010 12:05:54 +0100
To: Jan Dubois <jand [...] activestate.com>
From: Tim Bunce <Tim.Bunce [...] pobox.com>
Download (untitled) / with headers
text/plain 1.7k
On Wed, May 05, 2010 at 12:27:02PM -0700, Jan Dubois wrote: Show quoted text
> On Wed, 05 May 2010, kmx via RT wrote:
> > I see comming deadline for 5.12.1. > > > > Could somebody competent consider including 634b482c + 20c8f8f > > to 5.12.1
> > 634b482c is already in maint-5.12 as 72c7417: > > http://perl5.git.perl.org/perl.git/commitdiff/72c7417 > > 20c8f8f has been requested (by me) and got a second vote from Vincent. > It requires one other person with commit access to vote for it to > be eligible for merging. > > Given that this bug was detected with Devel::NYTProf, maybe Tim is > willing to put his vote in for this one (it is a single line addition > to XSUB.h).
It certainly gets a nod from me. (Strictly speaking I don't have a vote as I never did sort out commit rights to the new repository.) Tim. Show quoted text
> Cheers, > -Jan > > commit 20c8f8f9118fd23081c818637815bf1aab60b808 > Author: Jan Dubois <jand@activestate.com> > Date: Wed Apr 21 16:49:09 2010 -0700 > > XSUB.h is supposed to redefine fgets under PERL_IMPLICIT_SYS, but doesn't. > > See also http://rt.cpan.org/Public/Bug/Display.html?id=55049 > with workaround in http://code.google.com/p/perl-devel-nytprof/source/detail?r=1168 > > diff --git a/XSUB.h b/XSUB.h > index f23df37..06cb1c3 100644 > --- a/XSUB.h > +++ b/XSUB.h > @@ -507,6 +507,7 @@ Rethrows a previously caught exception. See L<perlguts/"Exception > # define ferror PerlSIO_ferror > # define clearerr PerlSIO_clearerr > # define getc PerlSIO_getc > +# define fgets PerlSIO_fgets > # define fputc PerlSIO_fputc > # define fputs PerlSIO_fputs > # define fflush PerlSIO_fflush > >
CC: Jan Dubois <jand [...] activestate.com>, perlbug-followup [...] perl.org, perl5-porters [...] perl.org
Subject: Re: [perl #72704] Buggy fputs(f,s) vs. fputs(s,f) on Win32 (maybe not only)
Date: Thu, 6 May 2010 08:21:40 -0500
To: Tim Bunce <Tim.Bunce [...] pobox.com>
From: "Craig A. Berry" <craig.a.berry [...] gmail.com>
Download (untitled) / with headers
text/plain 1020b
On Thu, May 6, 2010 at 6:05 AM, Tim Bunce <Tim.Bunce@pobox.com> wrote: Show quoted text
> On Wed, May 05, 2010 at 12:27:02PM -0700, Jan Dubois wrote:
>> On Wed, 05 May 2010, kmx via RT wrote:
>> > I see comming deadline for 5.12.1. >> > >> > Could somebody competent consider including 634b482c + 20c8f8f >> > to 5.12.1
>> >> 634b482c is already in maint-5.12 as 72c7417: >> >>    http://perl5.git.perl.org/perl.git/commitdiff/72c7417 >> >> 20c8f8f has been requested (by me) and got a second vote from Vincent. >> It requires one other person with commit access to vote for it to >> be eligible for merging. >> >> Given that this bug was detected with Devel::NYTProf, maybe Tim is >> willing to put his vote in for this one (it is a single line addition >> to XSUB.h).
> > It certainly gets a nod from me. (Strictly speaking I don't have a vote > as I never did sort out commit rights to the new repository.)
20c8f8f is now in maint-5.12 as: <http://perl5.git.perl.org/perl.git/commitdiff/e560917618a7d70a9f3420f75c62e08872bf97b5>


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