Dave,

Your patch looks reasonable but has a few issues which need to be
addressed.

It doesn't address the setting of logsys_subsys_id but defines it.  I
want to avoid the situation where logsys_subsys_id is defined, but then
not set.  What I suggest here is to set logsys_subsys_id to some known
value (-1) and assert if that the subsystem id is that value within
log_printf to help developers catch this scenario.  At the moment the
current API enforces proper behavior (it wont link if the developer does
the wrong thing).  With your patch it will link, but may not behave
properly sending log messages to the wrong subsystem (0) instead of the
subsystem desired by the developer.  This is why the macros are there
(to set the subsystem id and define it).  Your patch addresses the
removal of the definition to a generic location but doesn't address at
all the setting of the subsystem id.

The logsys_exit function doesn't need to be added.  Instead this is
managed by logsys_atsegv and logsys_flush.  If you desire, you can keep
logsys_exit and have it call logsys_flush which is the proper thing to
do on exit.

Please follow the coding style guidelines (ie: match the rest of the
logsys code) so I don't have to rework your patch before commit.

Regards
-steve


What this means is that it could cause On Tue, 2008-07-01 at 14:32
-0500, David Teigland wrote: 
> The attached patch adds a simple, function-based api to logsys, allowing
> simple programs to use it more cleanly (without macros).
> 
> Dave
> 
> _______________________________________________
> Openais mailing list
> [email protected]
> https://lists.linux-foundation.org/mailman/listinfo/openais

_______________________________________________
Openais mailing list
[email protected]
https://lists.linux-foundation.org/mailman/listinfo/openais

Reply via email to