[PATCH 1/4 v2] os/log: Pull LogMessageTypeVerbString out of LogVMessageVerb

[Guillem Jover]

On Mon, 2011-08-01 at 12:21:20 +0800, Daniel Kurtz wrote:
> Also, optimize how the type and format strings are combined.
> 
> Signed-off-by: Daniel Kurtz <djkurtz at chromium.org>
> ---
>  os/log.c |  111 ++++++++++++++++++++++++++++++++++---------------------------
>  1 files changed, 62 insertions(+), 49 deletions(-)
> 
> diff --git a/os/log.c b/os/log.c
> index 4a310e6..06ae027 100644
> --- a/os/log.c
> +++ b/os/log.c
> @@ -323,58 +326,68 @@ LogWrite(int verb, const char *f, ...)
>      va_end(args);
>  }
>  
> -void
> -LogVMessageVerb(MessageType type, int verb, const char *format, va_list args)
> +/* Returns the Message Type string to prepend to a logging message, or NULL
> + * if the message will be dropped due to insufficient verbosity. */
> +static const char*
> +LogMessageTypeVerbString(MessageType type, int verb)
>  {
[...]
> +    switch (type) {
> +    case X_PROBED:
> +	return X_PROBE_STRING;
> +    case X_CONFIG:
> +	return  X_CONFIG_STRING;
> +    case X_DEFAULT:
> +	return  X_DEFAULT_STRING;

There's some spaces too many here.

> +    case X_CMDLINE:
> +	return X_CMDLINE_STRING;
> +    case X_NOTICE:
> +	return X_NOTICE_STRING;
> +    case X_ERROR:
> +	return X_ERROR_STRING;
> +    case X_WARNING:
> +	return X_WARNING_STRING;
> +    case X_INFO:
> +	return X_INFO_STRING;
> +    case X_NOT_IMPLEMENTED:
> +	return X_NOT_IMPLEMENTED_STRING;
> +    case X_UNKNOWN:
> +	return X_UNKNOWN_STRING;
> +    case X_NONE:
> +	return X_NONE_STRING;
>      }
> +    return X_UNKNOWN_STRING;
> +}
> +
> +void
> +LogVMessageVerb(MessageType type, int verb, const char *format, va_list args)
> +{
> +    const char *type_str;
> +    char tmpFormat[1024];
> +    char *tmpFormat_end = &tmpFormat[sizeof(tmpFormat)];
> +    char *p = tmpFormat;
> +    int left;
> +
> +    type_str = LogMessageTypeVerbString(type, verb);
> +    if (!type_str)
> +	return;
> +
> +    /* if type_str is not "", prepend it, and ' ', to format */
> +    left = tmpFormat_end - p;
> +    if (strlen(type_str))
> +	p += snprintf(p, left, "%s ", type_str);
> +
> +    /* if still room in tmpBuf copy as much of format as will fit */
> +    left = tmpFormat_end - p;
> +    if (left > 1)
> +	snprintf(p, left, "%s", format);
> +
> +    LogVWrite(verb, tmpFormat, args);

How about something like this instead:

    const char *type_str;
    char tmpFormat[1024];
    char *new_format;

    type_str = LogMessageTypeVerbString(type, verb);
    if (!type_str)
        return;

    if (type_str[0] == '\0')
        new_format = format;
    else
    {
        new_format = tmpFormat;
        snprintf(tmpFormat, sizeof(tmpFormat), "%s %s", type_str, format);
    }

    LogVWrite(verb, new_format, args);
>  }
>  
>  /* Log message with verbosity level specified. */

regards,
guillem