Hi, On Mon, Nov 15, 2010 at 03:27:39PM +0100, Bernd Schubert wrote: > Hello Lars, > > firstly thanks a lot for your super fast review and reply. I will send > updated > patches on Wednesday. > > On Friday, November 12, 2010, Lars Ellenberg wrote: > > On Fri, Nov 12, 2010 at 05:04:39PM +0100, [email protected] wrote: > > > # HG changeset patch > > > # User Bernd Schubert <[email protected]> > > > # Date 1289577717 -3600 > > > # Node ID 39cc45ad8013fbf725254974070407fee2e1027b > > > # Parent ad13ee71cc6672092b13a73e89c7b0894faa5146 > > > cl_log: Simplify a function > > > > > > Signed-off-by: Bernd Schubert <[email protected]> > > > > > > diff --git a/lib/clplumbing/cl_log.c b/lib/clplumbing/cl_log.c > > > --- a/lib/clplumbing/cl_log.c > > > +++ b/lib/clplumbing/cl_log.c > > > @@ -534,7 +534,7 @@ cl_direct_log(int priority, const char* > > > > > > entity =cl_log_entity; > > > > > > } > > > > > > - pristr = use_priority_str ? prio2str(priority) : NULL; > > > + pristr = use_priority_str ? prio2str(priority) : ""; > > > > > > if (needprivs) { > > > > > > return_to_orig_privs(); > > > > > > @@ -544,17 +544,13 @@ cl_direct_log(int priority, const char* > > > > > > if (entity) { > > > > > > strncpy(common_log_entity, entity, MAXENTITY); > > > > > > } else { > > > > > > - strncpy(common_log_entity, DFLT_ENTITY,MAXENTITY); > > > + strncpy(common_log_entity, DFLT_ENTITY, MAXENTITY); > > > > > > } > > > > > > common_log_entity[MAXENTITY-1] = '\0'; > > > > > > - if (pristr) { > > > - syslog(priority, "[%d]: %s: %s%c", > > > - entity_pid, pristr, buf, 0); > > > - }else { > > > - syslog(priority, "[%d]: %s%c", entity_pid, buf, 0); > > > - } > > > + syslog(priority, "[%d]: %s: %s%c", > > > + entity_pid, pristr, buf, 0); > > > > This leaves an ugly colon. "[1234]: : message here" > > Why? > > What's wrong with an extra test on NULL > > for the use_priority_str == 0 case? > > I would like to avoid two times almost the same syslog() line if possible. > And > I didn't mind "[1234]: : message here"... > So we have two choices - either drop the patch, or modify the patch, keep the > if-condition and already add ": " there to pristr. Keeping 'if', we could > remove the initial pristr assignment.
I'd just drop the patch. It's not that bad as it is. Appending ": " would just complicate matter. BTW, anybody knows what's the purpose of adding '\0' to the end of the message? As far as I can see we can remove that. Cheers, Dejan > Cheers, > Bernd > _______________________________________________________ > Linux-HA-Dev: [email protected] > http://lists.linux-ha.org/mailman/listinfo/linux-ha-dev > Home Page: http://linux-ha.org/ _______________________________________________________ Linux-HA-Dev: [email protected] http://lists.linux-ha.org/mailman/listinfo/linux-ha-dev Home Page: http://linux-ha.org/
