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:
PyFloat_FromDouble(time*0.000001)
where time is also apr_time_t.
Jim
