[mod_python] quick code review

Greg Stein gstein at lyra.org
Thu May 25 19:12:10 EST 2000


There are a number of issues that I see with mod_python.c. I'll try to
address a good number of them here, but I'm invariably going to miss some.
After some changes are made, then another pass can be done at that point.

Please try to take this constructively... the goal is to improve the
code... :-)

*) when storing the tstate into obcallback, use a PyCObject rather than an
   integer. That will avoid the (int) casting (a no-no)

*) noop is not needed; use ap_null_cleanup() (I think that's the name;
   close to it, at least)

*) simplify make_obcallback(): the two params are always NULL

*) I don't understand why the PyTypeObject's are init'd/copied in
   python_init, rather than just doing that at the module level. For
   example:
     static PyTypeObject serverobjecttype = { ... };

*) use PyImport_Import() rather than PyRun_SimpleString to get the
   module imported. This returns the "mod_python" or "mod_python.apache"
   module (I forget which; probably the former).

*) use PyObject_CallMethod() to call the "init" method in the module
   retrieved from above. Use the *return* value for obCallBack, rather
   than the SetCallback nastiness.

*) in python_directive, just use a cast to (py_dir_config *) rather than
   all the offset stuff. Also, there is no need to dup() the string since
   it is a constant.

*) unused: python_dict_merge, table2dict, print_table, print_array

*) exit(1) should not be used; use ap_log_rerror() and return failure
   codes to Apache

*) rather than storing the "request object" into r->notes, place it into
   tstate->dict. you won't have to do the atoi() stuff (which is Bad
   anyways)

*) use ap_log_rerror() instead of ap_log_error(). the former is better
   from a logging standpoint (it has more info)


That's all for now!

Cheers,
-g

-- 
Greg Stein, http://www.lyra.org/
_______________________________________________
Mod_python mailing list
Mod_python at modpython.org
http://www.modpython.org/mailman/listinfo/mod_python




More information about the Mod_python mailing list