You are viewing a plain text version of this content. The canonical link for it is here.
Posted to modperl@perl.apache.org by NormW <no...@gknw.net> on 2013/04/19 10:15:58 UTC

mod_perl-2.0.8 + OWC

hi,
Some diffs you may also not be interested in when compiling MP 2.0.8 
with the OpenWatcom compiler:

1. OWC (ATM) doesn't have noreturn; a hack based on a Perl macro.
> --- modperl_config.c.orig	2011-02-03 07:23:45.000000000 +1100
> +++ modperl_config.c	2011-08-14 09:33:58.937500000 +1000
> @@ -659,3 +659,4 @@
>          }
>      }
>
> +     NORETURN_FUNCTION_END;
>  }


2. A NetWare hack.
> --- modperl_xsinit.c.orig	2008-09-07 13:44:46.218750000 +1000
> +++ modperl_xsinit.c	2010-10-07 19:41:12.078125000 +1000
> @@ -4,7 +4,6 @@
>  EXTERN_C void xs_init (pTHX);
>
>  EXTERN_C void boot_DynaLoader (pTHX_ CV* cv);
> -EXTERN_C void boot_Win32CORE (pTHX_ CV* cv);
>
>  EXTERN_C void
>  xs_init(pTHX)
> @@ -14,5 +13,4 @@
>
>  	/* DynaLoader is a special case */
>  	newXS("DynaLoader::boot_DynaLoader", boot_DynaLoader, file);
> -	newXS("Win32CORE::bootstrap", boot_Win32CORE, file);


3. In my build len == size_t == uint; this stifles a warning when len 
CAN'T go below zero.
> --- modperl_io_apache.c.orig	2011-02-03 07:23:45.000000000 +1100
> +++ modperl_io_apache.c	2011-08-14 08:48:46.531250000 +1000
> @@ -256,7 +256,7 @@
>      char *tmp = buffer;
>      apr_bucket_brigade *bb;
>
> -    if (len <= 0) {
> +    if (len < 1) {
>          return 0;
>      }


4. A NetWare hack.
> --- modperl_flags.h.orig	2008-09-07 13:44:46.250000000 +1000
> +++ modperl_flags.h	2008-09-08 13:55:19.828125000 +1000
> @@ -12,8 +12,7 @@
>   * 04: makefile.pl:96
>   */
>
> -
> -#define MP_SYS_DL_WIN32 1
> +#define MP_SYS_DL_DLOPEN 1
>
>  #define MpDirSeenFLAGS(p) (p)->flags->opts_seen


5. Stifle a compiler warning about a function mismatch.
> --- modperl_common_util.c.orig	2011-02-08 13:00:10.000000000 +1100
> +++ modperl_common_util.c	2012-04-26 11:33:16.152793900 +1000
> @@ -41,7 +41,7 @@
>
>  MP_INLINE static
>  int modperl_table_magic_copy(pTHX_ SV *sv, MAGIC *mg, SV *nsv,
> -                             const char *name, int namelen)
> +                             const char *name, I32 namelen)
>  {
>      /* prefetch the value whenever we're iterating over the keys */
>      MAGIC *tie_magic = mg_find(nsv, PERL_MAGIC_tiedelem);


6. Same as 3.
> --- Apache2__RequestIO.h.orig	2013-03-15 07:14:44.390625000 +1100
> +++ Apache2__RequestIO.h	2013-03-25 08:53:38.859375000 +1100
> @@ -248,8 +248,8 @@
>
>      (void)SvPV_force(buffer, blen); /* make it a valid PV */
>
> -    if (len <= 0) {
> -        Perl_croak(aTHX_ "The LENGTH argument can't be negative");
> +    if (len < 1) {
> +        Perl_croak(aTHX_ "The LENGTH argument can't be < +1");
>      }
>
>      /* handle negative offset */


7. The compiler liked this initialised.
> --- modperl_apr_perlio.c.orig	2012-07-22 07:51:04.140625000 +1000
> +++ modperl_apr_perlio.c	2013-03-25 08:58:30.187500000 +1100
> @@ -547,7 +547,7 @@
>  {
>      MP_IO_TYPE *retval;
>      char *mode;
> -    int fd;
> +    int fd = 0;
>      apr_os_file_t os_file;
>      apr_status_t rc;
>


8. NetWare doesn't have mmap support as set by APR build flags.
> --- Response.xs.orig	2008-09-07 13:44:34.312500000 +1000
> +++ Response.xs	2008-09-16 11:00:04.375000000 +1000
> @@ -73,22 +73,6 @@
>
>  MODULE = Apache2::Response    PACKAGE = Apache2::RequestRec   PREFIX = ap_
>
> -size_t
> -ap_send_mmap(r, mm, offset, length)
> -    Apache2::RequestRec r
> -    APR::Mmap mm
> -    size_t offset
> -    size_t length
> -
> -
> -    CODE:
> -    RETVAL = ap_send_mmap(mm, r, offset, length);
> -
> -    OUTPUT:
> -    RETVAL
> -
> -MODULE = Apache2::Response    PACKAGE = Apache2::RequestRec   PREFIX = ap_
> -
>  void
>  ap_set_content_length(r, length=r->finfo.csize)
>      Apache2::RequestRec r

Otherwise builds nicely; saving testing for perl 5.18.0.
N.

Re: mod_perl-2.0.8 + OWC

Posted by NormW <no...@gknw.net>.
On 22/04/2013 8:48 PM, Phil Carmody wrote:
>> 7. The compiler liked this initialised.
>>> --- modperl_apr_perlio.c.orig
>> 2012-07-22 07:51:04.140625000 +1000
>>> +++ modperl_apr_perlio.c    2013-03-25
>> 08:58:30.187500000 +1100
>>> @@ -547,7 +547,7 @@ { MP_IO_TYPE *retval; char *mode; -    int
>>> fd; +    int fd = 0; apr_os_file_t os_file; apr_status_t rc;
>
>
> No. The compiler didn't like that initialised, the compiler didn't
> like the use of it uninitialised, which was here:
>
> if (!(retval = PerlIO_fdopen(os_file, mode))) { PerlLIO_close(fd);
> Perl_croak(aTHX_ "fdopen failed!"); }
>
> Where did 0 come from in your patch? Why not 42? Why not 666? Why is
> it so important to you to close file descriptor 0 when that
> PerlIO_fdopen fails?
>
> What you seem to have done is remove the useful warning to a
> potential bug. I.e. you've made the situation worse.
>
> I have never looked at the MP codebase before, but from very quick
> inspection, the correct fix is to comment out the definition of fd
> and that use of it. I lie. The correct thing to do is to delete the
> references to it, and the commented out code. Version control systems
> are how you should keep the historical versions accessible, not
> comments.
>
> Phil
Hi,
And thanks for the meta-physical boot to have a closer look myself. I 
used to compile Perl and Mod_Perl with an old Metrowerks compiler and it 
used to bark whenever anything wasn't initialised before using, even it 
seemed when on the left-hand side. When OWC did the same I must have 
just added the '= 0' as a (in a?) hangover, without checking the code, 
and of course the warning disappeared, so took it as all is well.

Based on fd non-usage in the present code, and commented out from at 
least 2.0.4, I'll revise my patch to remove fd as you suggest.
Thanks for the feedback.

Norm

Re: mod_perl-2.0.8 + OWC

Posted by Phil Carmody <th...@yahoo.co.uk>.
> 7. The compiler liked this initialised.
> > --- modperl_apr_perlio.c.orig   
> 2012-07-22 07:51:04.140625000 +1000
> > +++ modperl_apr_perlio.c    2013-03-25
> 08:58:30.187500000 +1100
> > @@ -547,7 +547,7 @@
> >  {
> >      MP_IO_TYPE *retval;
> >      char *mode;
> > -    int fd;
> > +    int fd = 0;
> >      apr_os_file_t os_file;
> >      apr_status_t rc;


No. The compiler didn't like that initialised, the compiler didn't like the use of it uninitialised, which was here:

    if (!(retval = PerlIO_fdopen(os_file, mode))) { 
        PerlLIO_close(fd);
        Perl_croak(aTHX_ "fdopen failed!");
    } 

Where did 0 come from in your patch? Why not 42? Why not 666? Why is it so important to you to close file descriptor 0 when that PerlIO_fdopen fails?

What you seem to have done is remove the useful warning to a potential bug. I.e. you've made the situation worse.

I have never looked at the MP codebase before, but from very quick inspection, the correct fix is to comment out the definition of fd and that use of it. I lie. The correct thing to do is to delete the references to it, and the commented out code. Version control systems are how you should keep the historical versions accessible, not comments.

Phil
-- 
()  ASCII ribbon campaign      ()    Hopeless ribbon campaign
/\    against HTML mail        /\  against gratuitous bloodshed

[stolen with permission from Daniel B. Cristofani]