Hi Ginnie,
I'm reviewing the logging code, but would like to first raise one general point
for discussion, and that is the use of PyObject in the exported API.
I think that the fact that this is a library over Python shouldn't be exposed
to the consumer of the API - so essentially anything in the C API headers
shouldn't ever reference PyObject*, but should instead be using something like
"logging_handle_t", similar to what we did in the error service, so methods
like:
extern PyObject *create_filehandler(nvlist_t *);
extern PyObject *create_progresshandler(nvlist_t *);
extern PyObject *create_httphandler(nvlist_t *hdlrlst);
extern PyObject *create_streamhandler(nvlist_t *hdlrlst);
would be better as:
extern logging_handle_t create_filehandler(nvlist_t *);
extern logging_handle_t create_progresshandler(nvlist_t *);
extern logging_handle_t create_httphandler(nvlist_t *hdlrlst);
extern logging_handle_t create_streamhandler(nvlist_t *hdlrlst);
and similarly for methods then like:
boolean_t transfer_log(PyObject *, char *);
which would be better as:
boolean_t transfer_log(logging_handle_t, char *);
The type would be defined as :
typedef void* logging_handle_t;
and in each of you're methods you would do:
PyObject *_logger = (PyObject*)handle;
This in turn means all the consumers of the API wouldn't be required to import
Python.h, via the logging header files.
What do you think?
Thanks,
Darren.
On 07/ 9/10 07:33 PM, Ginnie Wray wrote:
> Hi -
>
> I want to get my code review started, and I still have a little
> cleanup to do.
> I'm going to generate my code review in two parts with the
> first part the C bridge code. I haven't quite got the test code
> for the C cleaned up, so I'll present that with the second part as well.
>
> Pointer to the webrev:
> http://cr.opensolaris.org/~ginnie/logging_Cfiles/
>
>
> I would like to have feedback by July 23rd COB.
>
> Thanks,
>
> ginnie
> _______________________________________________
> caiman-discuss mailing list
> [email protected]
> http://mail.opensolaris.org/mailman/listinfo/caiman-discuss
_______________________________________________
caiman-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/caiman-discuss
