Hi Ginnie,
Here are my comments on the C files.
usr/src/lib/install_logging/handlers.c
-----------------------------------------
lines 65-70: The value for pFileDict won't be used until line 86. Why not
move these few lines down closer to where it will be used.
lines 166-178: This block does not need to be in an else statement.
The "if" will catch the failure case.
The create_httphandler() and create_streamhandler() functions in this
file will not have the corresponding back-end support in Python. Do we
really want to have the code here now?
usr/src/lib/install_logging/install_logging.c
-----------------------------------------------
lines 200-201: To improve readability of the code, I think it is better
to give more meaningful names to these variables. What about
pModuleLogging and pModuleLogsvc
line 221: do we need to check whether "pDict" is NULL before using it?
line 254: This line will get executed even if there's no error. According
to Python document, it will cause a fatal error.
line 276: The Python version of this function allows users to pass in
a "source". Are we not going to allow that functionality in the C version?
line 487: I think it is more descriptive to name this function
"set_log_level"
line 512-513, 660-661: Same comment as above about the 2 variable names
line 854, 949: debug messages should be removed.
usr/src/lib/install_logging/install_logging.h
-----------------------------------------------
line 44: socket handler is never used. Should not be defined.
Thanks,
--Karen
On 07/ 9/10 11:33 AM, 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
