Great work! comments that should be addressed: logsys.h:
please no single line comments, especially in header files. can you multiline this or put it in the comment above the definitions? /* FORK and THREADED are ignored for SUBSYSTEMS */ How would you feel about changing LOG_MODE_* to LOGSYS_MODE_*? I'd prefer all defines in public headers use LOGSYS as starters. This doesn't apply to LOG_LEVEL unfortunately because that change would be huge. /* logsys_logger bits */ #define SUBSYS_MAX 63 /* maximum number of subsystems */ #define SUBSYS_MAXLEN 64 /* max len of a subsystem name */ ^^single line comments are evil imo please fix If this is going to be in a public header, it should be prefixed with LOGSYS_ (such as LOGSYS_SUBSYS_MAX). can you change LOG_REC_END to LOGSYS_REC_END? logsys.c: /* similar to syslog facilities/priorities tables, * make a tag table for internal use */ ^ improper comment style tagnames has improper tabbing (line 75). Really great work. If you like, please add your name to the license section of the header and c file. regards -steve On Wed, 2009-04-15 at 09:52 +0200, Fabio M. Di Nitto wrote: > Hi guys, > > changes compared to take2: > > - move SUBSYS_MAX and SUBSYS_MAXLEN in logsys.h. The former is required > to assert within DECLARE_* and the latter is good to have visibile as > it's part of the public API limit for subsystem name. > > - make DECLARE_* macros assert on error. > > - fix error handling in _logsys_rec_init by returning instead of > asserting and remove duplicated assertion (there are a few more of > those, but as noted in take2, now we can handle everything internally). > > - fix LOG_MODE_THREADED to not fire wthread when working in non-threaded > mode. > > This version has been tested with a full corosync run and it seems to > handle properly. > > As soon as this one is ACK, I'll post the rest of corosync port to the > new API. > > Fabio > _______________________________________________ > 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
