On Wed, 2009-04-01 at 13:31 +0200, Fabio M. Di Nitto wrote:
> On Wed, 2009-04-01 at 13:15 +0200, Fabio M. Di Nitto wrote:
> > On Wed, 2009-04-01 at 11:09 +0200, Fabio M. Di Nitto wrote:
> > > On Wed, 2009-04-01 at 10:40 +0200, Fabio M. Di Nitto wrote:
> > > > Hi,
> > > >
> > > > this patch replaces the one posted the 31st Of March and it's going to
> > > > be my last take on it. I think I finally found nirvana and imho good
> > > > enough to be out there :)
> > > >
> > > > - add insert_into_buffer static function. This is a big helper to add
> > > > new logsys parameters anywhere in the current format_buffer.
> > > > It does some string manipulation that could probably be written to be
> > > > faster, but given that this code is executed only at startup and on
> > > > config reload, it will do.
> > > >
> > > > - move new_format_buffer to be more global within the same function so
> > > > it can be reused.
> > > >
> > > > - add support for fileline (as documented) and function_name (new).
> > > >
> > > > - simplify a lot also the timeline implementation.
> > > >
> > > > - make sure to reset the format buffer on reload. This is required to
> > > > set the new options properly (since we do not allow settings of custom
> > > > formats, it's the simplest way to achieve removal).
> > > >
> > > > - update corosync.conf.5 to add function_name.
> > > >
> > > > As a general benefit, the new function insert_into_buffer, remove the
> > > > requirement to parse the options in a specific order, but some care
> > > > still need to be taken to set the "after" bit in case of insert new
> > > > tokens into the format buffer.
> > >
> > > A few little changes after Jim's review on IRC:
> > >
> > > - make current_format const
> > > - move templen, afterpos and afterlen down in the block where they are
> > > used.
> > > - strstr should check against != NULL and not > 0.
> >
> > A few more changes after another review from Jim on IRC:
> >
> > - document insert_into_buffer function to make sure it's not misused.
> > - add bufferlen to insert_into_buffer call.
> > - return -1 if we are truncating in snprintf operations.
> > - make afterpos const.
> >
> > Extra bonus:
> > - fix return value if entry is already part of current format string.
> >
> > Fabio
> >
> > NOTE for me: At some point in your life you realize that nirvana doesn't
> > exist ;)))
>
> Another round of changes from Jim review:
>
> - stop polluting with memset.
- Fix wrong bufferlen checks in snprintf
Fabio
Index: exec/mainconfig.c
===================================================================
--- exec/mainconfig.c (revision 1975)
+++ exec/mainconfig.c (working copy)
@@ -105,6 +105,78 @@
}
}
+
+/**
+ * insert_into_buffer
+ * @target_buffer: a buffer where to write results
+ * @bufferlen: tell us the size of the buffer to avoid overflows
+ * @entry: entry that needs to be added to the buffer
+ * @after: can either be NULL or set to a string.
+ * if NULL, @entry is prependend to logsys_format_get buffer.
+ * if set, @entry is added immediately after @after.
+ *
+ * Since the function is specific to logsys_format_get handling, it is implicit
+ * that source is logsys_format_get();
+ *
+ * In case of failure, target_buffer could be left dirty. So don't trust
+ * any data leftover in it.
+ *
+ * Searching for "after" assumes that there is only entry of "after"
+ * in the source. Afterall we control the string here and for logging format
+ * it makes little to no sense to have duplicate format entries.
+ *
+ * Returns: 0 on success, -1 on failure
+ **/
+static int insert_into_buffer(
+ char *target_buffer,
+ size_t bufferlen,
+ const char *entry,
+ const char *after)
+{
+ const char *current_format = NULL;
+
+ current_format = logsys_format_get();
+
+ /* if the entry is already in the format we don't add it again */
+ if (strstr(current_format, entry) != NULL) {
+ return -1;
+ }
+
+ /* if there is no "after", simply prepend the requested entry
+ * otherwise go for beautiful string manipulation.... </sarcasm> */
+ if (!after) {
+ if (snprintf(target_buffer, bufferlen - 1, "%s%s",
+ entry,
+ current_format) >= bufferlen - 1) {
+ return -1;
+ }
+ } else {
+ const char *afterpos;
+ size_t afterlen;
+ size_t templen;
+
+ /* check if after is contained in the format
+ * and afterlen has a meaning or return an error */
+ afterpos = strstr(current_format, after);
+ afterlen = strlen(after);
+ if ((!afterpos) || (!afterlen)) {
+ return -1;
+ }
+
+ templen = afterpos - current_format + afterlen;
+ if (snprintf(target_buffer, templen + 1, "%s", current_format)
+ >= bufferlen - 1) {
+ return -1;
+ }
+ if (snprintf(target_buffer + templen, bufferlen - ( templen + 1 ),
+ "%s%s", entry, current_format + templen)
+ >= bufferlen - ( templen + 1 )) {
+ return -1;
+ }
+ }
+ return 0;
+}
+
static struct logsys_config_struct {
char subsys[6];
unsigned int priority;
@@ -122,6 +194,7 @@
const char *error_reason = error_string_response;
hdb_handle_t object_find_handle;
hdb_handle_t object_find_logsys_handle;
+ char new_format_buffer[PATH_MAX];
objdb->object_find_create (
OBJECT_PARENT_HANDLE,
@@ -158,24 +231,58 @@
main_config->logmode &= ~LOG_MODE_OUTPUT_STDERR;
}
}
- if (!objdb_get_string (objdb,object_service_handle, "timestamp", &value)) {
- char new_format_buffer[PATH_MAX];
-
- memset(&new_format_buffer, 0, sizeof(new_format_buffer));
-
+ if (!objdb_get_string (objdb,object_service_handle, "fileline", &value)) {
if (strcmp (value, "on") == 0) {
- snprintf(new_format_buffer, PATH_MAX-1, "%%t %s", logsys_format_get());
- logsys_format_set(new_format_buffer);
+ if (!insert_into_buffer(new_format_buffer,
+ sizeof(new_format_buffer),
+ " %f:%l", "s]")) {
+ logsys_format_set(new_format_buffer);
+ } else
+ if (!insert_into_buffer(new_format_buffer,
+ sizeof(new_format_buffer),
+ "%f:%l", NULL)) {
+ logsys_format_set(new_format_buffer);
+ }
} else
if (strcmp (value, "off") == 0) {
- if (!strncmp("%t ", logsys_format_get(), 3)) {
- snprintf(new_format_buffer, PATH_MAX-1, "%s", logsys_format_get() + 3);
+ /* nothing to do here */
+ } else {
+ goto parse_error;
+ }
+ }
+ if (!objdb_get_string (objdb,object_service_handle, "function_name", &value)) {
+ if (strcmp (value, "on") == 0) {
+ if (!insert_into_buffer(new_format_buffer,
+ sizeof(new_format_buffer),
+ "%n:", "f:")) {
logsys_format_set(new_format_buffer);
+ } else
+ if (!insert_into_buffer(new_format_buffer,
+ sizeof(new_format_buffer),
+ " %n", "s]")) {
+ logsys_format_set(new_format_buffer);
}
+ } else
+ if (strcmp (value, "off") == 0) {
+ /* nothing to do here */
} else {
goto parse_error;
}
}
+ if (!objdb_get_string (objdb,object_service_handle, "timestamp", &value)) {
+ if (strcmp (value, "on") == 0) {
+ if(!insert_into_buffer(new_format_buffer,
+ sizeof(new_format_buffer),
+ "%t ", NULL)) {
+ logsys_format_set(new_format_buffer);
+ }
+ } else
+ if (strcmp (value, "off") == 0) {
+ /* nothing to do here */
+ } else {
+ goto parse_error;
+ }
+ }
/* free old string on reload */
if (main_config->logfile) {
@@ -186,19 +293,6 @@
main_config->logfile = strdup (value);
}
- if (!objdb_get_string (objdb,object_service_handle, "fileline", &value)) {
-/* TODO
- if (strcmp (value, "on") == 0) {
- main_config->logmode |= LOG_MODE_DISPLAY_FILELINE;
- } else
- if (strcmp (value, "off") == 0) {
- main_config->logmode &= ~LOG_MODE_DISPLAY_FILELINE;
- } else {
- goto parse_error;
- }
-*/
- }
-
if (!objdb_get_string (objdb,object_service_handle, "syslog_facility", &value)) {
main_config->syslog_facility = logsys_facility_id_get(value);
if (main_config->syslog_facility < 0) {
@@ -392,6 +486,7 @@
/*
* Reload the logsys configuration
*/
+ logsys_format_set(NULL);
corosync_main_config_read_logging(global_objdb,
&error_string,
main_config);
Index: man/corosync.conf.5
===================================================================
--- man/corosync.conf.5 (revision 1975)
+++ man/corosync.conf.5 (working copy)
@@ -442,10 +442,16 @@
.TP
fileline
-This specifies that file and line should be printed instead of logger name.
+This specifies that file and line should be printed.
The default is off.
+.TP
+function_name
+This specifies that the code function name should be printed.
+
+The default is off.
+
.TP
syslog_facility
This specifies the syslog facility type that will be used for any messages
_______________________________________________
Openais mailing list
[email protected]
https://lists.linux-foundation.org/mailman/listinfo/openais
