You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@perl.apache.org by Stas Bekman <st...@stason.org> on 2003/03/14 03:10:53 UTC
Re: [mp1] warnings patches
Randy Kobes wrote:
> Just for completeness, attached is a diff against the
> src/modules/perl mod_perl 1.0 cvs directory which gets rid of
> (all but one) warning on Win32. There's a few typecast
> insertions, plus some avoiding declaring variables that won't be
> used for Win32. One thing that I wasn't quite sure about the
> portability - "old_warn", in, eg, perl_util.c, was defined as
> I32, but Windows preferred it to be unsigned char.
see below:
> Index: src/modules/perl/Apache.xs
> ===================================================================
> RCS file: /home/cvspublic/modperl/src/modules/perl/Apache.xs,v
> retrieving revision 1.125
> diff -u -r1.125 Apache.xs
> --- src/modules/perl/Apache.xs 6 Jul 2001 20:33:35 -0000 1.125
> +++ src/modules/perl/Apache.xs 23 Jan 2003 05:18:52 -0000
> @@ -1002,7 +1002,7 @@
>
> if (should_client_block(r)) {
> (void)SvUPGRADE(buffer, SVt_PV);
> - SvGROW(buffer, bufsiz+1);
> + SvGROW(buffer, (STRLEN) bufsiz+1);
> nrd = get_client_block(r, SvPVX(buffer), bufsiz);
> }
> r->read_length += old_read_length;
> @@ -1041,7 +1041,7 @@
>
> PPCODE:
> (void)SvUPGRADE(buffer, SVt_PV);
> - SvGROW(buffer, bufsiz+1);
> + SvGROW(buffer, (STRLEN) bufsiz+1);
> nrd = get_client_block(r, SvPVX(buffer), bufsiz);
> if ( nrd > 0 ) {
> XPUSHs(sv_2mortal(newSViv((long)nrd)));
should we instead change the declaration of bufsiz to
STRLEN bufsize;
> Index: src/modules/perl/Constants.xs
> ===================================================================
> RCS file: /home/cvspublic/modperl/src/modules/perl/Constants.xs,v
> retrieving revision 1.20
> diff -u -r1.20 Constants.xs
> --- src/modules/perl/Constants.xs 30 Mar 2000 00:44:40 -0000 1.20
> +++ src/modules/perl/Constants.xs 23 Jan 2003 05:18:52 -0000
> @@ -906,7 +906,7 @@
> #endif
> char *name = EXP_NAME;
> double val = constant(name);
> - my_newCONSTSUB(stash, name, newSViv(val));
> + my_newCONSTSUB(stash, name, newSViv((I32) val));
> }
> }
>
> @@ -955,7 +955,7 @@
> if(errno != 0)
> croak("Your vendor has not defined Apache::Constants macro `%s'", name);
> else
> - my_newCONSTSUB(stash, name, newSViv(val));
> + my_newCONSTSUB(stash, name, newSViv((I32) val));
+1
> const char *
> SERVER_VERSION()
> Index: src/modules/perl/mod_perl.c
> ===================================================================
> RCS file: /home/cvspublic/modperl/src/modules/perl/mod_perl.c,v
> retrieving revision 1.145
> diff -u -r1.145 mod_perl.c
> --- src/modules/perl/mod_perl.c 23 May 2002 04:35:16 -0000 1.145
> +++ src/modules/perl/mod_perl.c 23 Jan 2003 05:18:53 -0000
> @@ -1099,8 +1099,11 @@
> static void per_request_cleanup(request_rec *r)
> {
> dPPREQ;
> +
> +#ifndef WIN32
> perl_request_sigsave **sigs;
> int i;
> +#endif
>
> if(!cfg) {
> return;
+1
> Index: src/modules/perl/mod_perl.h
> ===================================================================
> RCS file: /home/cvspublic/modperl/src/modules/perl/mod_perl.h,v
> retrieving revision 1.116
> diff -u -r1.116 mod_perl.h
> --- src/modules/perl/mod_perl.h 23 May 2002 04:35:16 -0000 1.116
> +++ src/modules/perl/mod_perl.h 23 Jan 2003 05:18:54 -0000
> @@ -1174,7 +1174,7 @@
> I32 perl_module_is_loaded(char *name);
> SV *perl_module2file(char *name);
> int perl_require_module(char *module, server_rec *s);
> -int perl_load_startup_script(server_rec *s, pool *p, char *script, I32 my_warn);
> +int perl_load_startup_script(server_rec *s, pool *p, char *script, unsigned char my_warn);
> array_header *perl_cgi_env_init(request_rec *r);
> void perl_clear_env(void);
> void mp_magic_setenv(char *key, char *val, int is_tainted);
I think that should be: s/I32/U8/
> Index: src/modules/perl/perl_config.c
> ===================================================================
> RCS file: /home/cvspublic/modperl/src/modules/perl/perl_config.c,v
> retrieving revision 1.114
> diff -u -r1.114 perl_config.c
> --- src/modules/perl/perl_config.c 24 Mar 2002 22:51:04 -0000 1.114
> +++ src/modules/perl/perl_config.c 23 Jan 2003 05:18:56 -0000
> @@ -392,7 +392,11 @@
>
> perl_request_config *perl_create_request_config(pool *p, server_rec *s)
> {
> +
> +#ifndef WIN32
> int i;
> +#endif
+1
> perl_request_config *cfg =
> (perl_request_config *)pcalloc(p, sizeof(perl_request_config));
> cfg->pnotes = Nullhv;
> @@ -1528,7 +1532,7 @@
> {
> I32 alen = AvFILL(av);
> I32 i, j;
> - I32 oldwarn = dowarn; /*XXX, hmm*/
> + unsigned char oldwarn = dowarn; /*XXX, hmm*/
> dowarn = FALSE;
U8?
> if(!n) n = alen+1;
> Index: src/modules/perl/perl_util.c
> ===================================================================
> RCS file: /home/cvspublic/modperl/src/modules/perl/perl_util.c,v
> retrieving revision 1.50
> diff -u -r1.50 perl_util.c
> --- src/modules/perl/perl_util.c 25 Dec 2002 01:46:09 -0000 1.50
> +++ src/modules/perl/perl_util.c 23 Jan 2003 05:18:56 -0000
> @@ -470,7 +470,7 @@
> dPSRV(s);
> HV *hash = GvHV(incgv);
> HE *entry;
> - I32 old_warn = dowarn;
> + unsigned char old_warn = dowarn;
> pool *p = ap_make_sub_pool(sp);
> table *reload = ap_make_table(p, HvKEYS(hash));
> char **entries;
> @@ -573,10 +573,10 @@
> /*(void)hv_delete(GvHV(incgv), pv, strlen(pv), G_DISCARD);*/
> }
>
> -int perl_load_startup_script(server_rec *s, pool *p, char *script, I32 my_warn)
> +int perl_load_startup_script(server_rec *s, pool *p, char *script, unsigned char my_warn)
> {
> dTHR;
> - I32 old_warn = dowarn;
> + unsigned char old_warn = dowarn;
U8, all above.
> if(!script) {
> MP_TRACE_d(fprintf(stderr, "no Perl script to load\n"));
> @@ -719,7 +719,7 @@
> errpv = SvPVX(errsv);
>
> for(i=0;i<=2;i++) {
> - if(i >= SvCUR(errsv))
> + if(i >= (int) SvCUR(errsv))
> break;
> if(isDIGIT(SvPVX(errsv)[i]))
> http_code++;
> @@ -738,7 +738,7 @@
> }
>
> /* nothin but 3 digits */
> - if(SvCUR(errsv) == http_code)
> + if((int) SvCUR(errsv) == http_code)
> return TRUE;
s/int i=0/STRLEN i=0/?
I think it's cleaner to use the right type in first place, instead of doing
casting, if possible.
__________________________________________________________________
Stas Bekman JAm_pH ------> Just Another mod_perl Hacker
http://stason.org/ mod_perl Guide ---> http://perl.apache.org
mailto:stas@stason.org http://use.perl.org http://apacheweek.com
http://modperlbook.org http://apache.org http://ticketmaster.com
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org
Re: [mp1] warnings patches
Posted by Stas Bekman <st...@stason.org>.
>> One thing that I wasn't quite sure about the
>> portability - "old_warn", in, eg, perl_util.c, was defined as
>> I32, but Windows preferred it to be unsigned char.
>> - I32 oldwarn = dowarn; /*XXX, hmm*/
>> + unsigned char oldwarn = dowarn; /*XXX, hmm*/
>> dowarn = FALSE;
>
>
> U8?
it's really a 'bool' in perl ;)
Please check if U8 does the trick.
__________________________________________________________________
Stas Bekman JAm_pH ------> Just Another mod_perl Hacker
http://stason.org/ mod_perl Guide ---> http://perl.apache.org
mailto:stas@stason.org http://use.perl.org http://apacheweek.com
http://modperlbook.org http://apache.org http://ticketmaster.com
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org
Re: [mp1] warnings patches
Posted by Stas Bekman <st...@stason.org>.
Randy Kobes wrote:
> On Fri, 14 Mar 2003, Stas Bekman wrote:
>
>
>>since its a STRLEN and it can't be negative, I think the right
>>type is UV. Not that it much matters, but since we perfect
>>things, let's have it perfect ;)
>>
>>I'll send a patch for B/typemap to p5p.
>
>
> That sounds good ... I'll change Apache/typemap to use T_UV.
> Thanks.
I've tested with 5.6.1 and 5.8.0, all seems to work fine with your recent
patches. I can't test 5.005_03, my perl build it borked :(
probably add an entry to Changes:
Several variable types were adjusted/casted to avoid warnings
during the compilation.
or similar, just in case someone will have a trouble with the latest cvs and
will look for a hint in the Changes file.
__________________________________________________________________
Stas Bekman JAm_pH ------> Just Another mod_perl Hacker
http://stason.org/ mod_perl Guide ---> http://perl.apache.org
mailto:stas@stason.org http://use.perl.org http://apacheweek.com
http://modperlbook.org http://apache.org http://ticketmaster.com
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org
Re: [mp1] warnings patches
Posted by Randy Kobes <ra...@theoryx5.uwinnipeg.ca>.
On Fri, 14 Mar 2003, Stas Bekman wrote:
> since its a STRLEN and it can't be negative, I think the right
> type is UV. Not that it much matters, but since we perfect
> things, let's have it perfect ;)
>
> I'll send a patch for B/typemap to p5p.
That sounds good ... I'll change Apache/typemap to use T_UV.
Thanks.
--
best regards,
randy
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org
Re: [mp1] warnings patches
Posted by Stas Bekman <st...@stason.org>.
Randy Kobes wrote:
> On Fri, 14 Mar 2003, Stas Bekman wrote:
>
>
>>Randy Kobes wrote:
>>
>>
>>>>>Index: src/modules/perl/Apache.xs
>>>>>+ SvGROW(buffer, (STRLEN) bufsiz+1);
>>>>
>>>>should we instead change the declaration of bufsiz to
>>>>
>>>>STRLEN bufsize;
>>>
>>>
>>>That worked out - I had to also add
>>> STRLEN T_IV
>>>to Apache/typemap.
>>
>>But I think it should be: T_UV
>>At least it's T_UV in the core 5.8.0's typemap (it wasn't there before)
>
>
> I had used the one under perl-5.8.0/ext/B/typemap, but it is
> defined as T_UV in, eg, lib/ExtUtils/typemap. For me, on Win32,
> with perl-5.6.1, either one has all tests passing.
since its a STRLEN and it can't be negative, I think the right type is UV. Not
that it much matters, but since we perfect things, let's have it perfect ;)
I'll send a patch for B/typemap to p5p.
__________________________________________________________________
Stas Bekman JAm_pH ------> Just Another mod_perl Hacker
http://stason.org/ mod_perl Guide ---> http://perl.apache.org
mailto:stas@stason.org http://use.perl.org http://apacheweek.com
http://modperlbook.org http://apache.org http://ticketmaster.com
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org
Re: [mp1] warnings patches
Posted by Randy Kobes <ra...@theoryx5.uwinnipeg.ca>.
On Fri, 14 Mar 2003, Stas Bekman wrote:
> Randy Kobes wrote:
>
> >>>Index: src/modules/perl/Apache.xs
> >>>+ SvGROW(buffer, (STRLEN) bufsiz+1);
> >>
> >>should we instead change the declaration of bufsiz to
> >>
> >>STRLEN bufsize;
> >
> >
> > That worked out - I had to also add
> > STRLEN T_IV
> > to Apache/typemap.
>
> But I think it should be: T_UV
> At least it's T_UV in the core 5.8.0's typemap (it wasn't there before)
I had used the one under perl-5.8.0/ext/B/typemap, but it is
defined as T_UV in, eg, lib/ExtUtils/typemap. For me, on Win32,
with perl-5.6.1, either one has all tests passing.
--
best regards,
randy
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org
Re: [mp1] warnings patches
Posted by Stas Bekman <st...@stason.org>.
Randy Kobes wrote:
>>>Index: src/modules/perl/Apache.xs
>>>+ SvGROW(buffer, (STRLEN) bufsiz+1);
>>
>>should we instead change the declaration of bufsiz to
>>
>>STRLEN bufsize;
>
>
> That worked out - I had to also add
> STRLEN T_IV
> to Apache/typemap.
But I think it should be: T_UV
At least it's T_UV in the core 5.8.0's typemap (it wasn't there before)
I'm testing with your latest commits.
__________________________________________________________________
Stas Bekman JAm_pH ------> Just Another mod_perl Hacker
http://stason.org/ mod_perl Guide ---> http://perl.apache.org
mailto:stas@stason.org http://use.perl.org http://apacheweek.com
http://modperlbook.org http://apache.org http://ticketmaster.com
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org
Re: [mp1] warnings patches
Posted by Randy Kobes <ra...@theoryx5.uwinnipeg.ca>.
On Fri, 14 Mar 2003, Stas Bekman wrote:
> Randy Kobes wrote:
> > Just for completeness, attached is a diff against the
> > src/modules/perl mod_perl 1.0 cvs directory which gets rid of
> > (all but one) warning on Win32.
>
> see below:
>
Thanks, Stas!
> > Index: src/modules/perl/Apache.xs
> > + SvGROW(buffer, (STRLEN) bufsiz+1);
>
> should we instead change the declaration of bufsiz to
>
> STRLEN bufsize;
That worked out - I had to also add
STRLEN T_IV
to Apache/typemap.
> > Index: src/modules/perl/Constants.xs
> > + my_newCONSTSUB(stash, name, newSViv((I32) val));
>
> +1
done ...
> > Index: src/modules/perl/mod_perl.c
> > +
> > +#ifndef WIN32
> > perl_request_sigsave **sigs;
> > int i;
> > +#endif
>
> +1
done ...
> > Index: src/modules/perl/mod_perl.h
> > +int perl_load_startup_script(server_rec *s, pool *p, char *script,
> > unsigned char my_warn);
> I think that should be: s/I32/U8/
That worked out - thanks.
> > Index: src/modules/perl/perl_config.c
> > +#ifndef WIN32
> > int i;
> > +#endif
>
> +1
done ...
> > + unsigned char oldwarn = dowarn; /*XXX, hmm*/
> U8?
yup ...
> > Index: src/modules/perl/perl_util.c
> > + unsigned char old_warn = dowarn;
> > +int perl_load_startup_script(server_rec *s, pool *p, char *script,
> > unsigned char my_warn)
> > + unsigned char old_warn = dowarn;
>
> U8, all above.
right again ...
> > + if(i >= (int) SvCUR(errsv))
> > + if((int) SvCUR(errsv) == http_code)
>
> s/int i=0/STRLEN i=0/?
that works, and the same for http_code.
> I think it's cleaner to use the right type in first place, instead of doing
> casting, if possible.
Good point ... thanks again ...
--
best regards,
randy
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org