quetz-mod_python-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Jim Gallacher <...@jgassociates.ca>
Subject Re: mod_python 3.3.0-dev-20061109 tests on Win32
Date Thu, 16 Nov 2006 01:22:28 GMT
Jeff Robbins wrote:
> There are a number of build warnings on Win32 that I have been looking 
> at. While some are no doubt windows-specific, some look to be generic C 
> programming errors.   Is there a goal of getting to a warning-free build?

I'm not sure if it's ever been said out loud, but sure, why not? That 
being said, gcc doesn't complain about most of the warnings shown in 
your attachment.

> Two changes to mod_python.h remove a number of the warnings.

Please avoid using C++ style comments. They have a habit of creeping 
into our code because they are so darn convenient, but we are trying to 
maintain our C purity. ;)

>  The first 
> change is for Win32, and involves having the right flavor of DL_IMPORT 
> in play.  DL_IMPORT is a deprecated pyport.h macro (from python) that 
> lets users of DLL functions get the correct external declaration while 
> implementors of these functions get the correct internal declaration.
> The second change is to fix the declaration of  mp_release_interpreter 
> which is a void, not a void *.  I doubt this matters but it is an error. 
> APR_DECLARE_OPTIONAL_FN(void, mp_release_interpreter, ());
> The remaining build warnings are attached in a txt file for anyone who 
> cares.  I assume many are innocuous, but there are things like 
> python_filter() and python_input_filter() using different types for 
> their readbytes argument that I don't understand.  Why should these two 
> functions declare this arg differently?

I think that may be a mistake. They should probably both use apr_off_t 
rather than apr_size_t, but I wouldn't want to make that change without 
tracing the path all the way through the code. There are a lot of these 
sorts of issues that I think we'll need to review in conjunction with 
the python2.5 / 64 bit support.

Several of your other warnings such as:

C:\work\mod_python-3.3.0-dev-20061109\src\util.c(170) : warning C4244: 
'function' : conversion from 'double' to 'long', possible loss of data

are the result of converting apr_time_t from microseconds to seconds:

         PyTuple_SET_ITEM(t, 9, PyInt_FromLong(f->ctime*0.000001));

In these cases your compiler is complaining needlessly. None the less, I 
think multiplying by 0.000001 is both sloppy and error prone for these 
types of conversions.

I think we should do something like this:

         PyTuple_SET_ITEM(t, 9,
                          PyInt_FromLong(f->ctime/ APR_USEC_PER_SEC));
or perhaps use the apr_time_sec macro:
	PyTuple_SET_ITEM(t, 9, PyInt_FromLong(apr_time_sec(f->ctime)));

On the other hand, maybe some of these conversions could be considered 
bugs. Should the mtime, ctime and atime of the finfo object be 
restricted to 1 second resolution? For comparison, req.request_time 
converts to a float:
where time is also apr_time_t.


View raw message