[PATCH 5/5] os: don't block signal-unsafe logging, merely warn about it.

[Chase Douglas]

On 08/15/2012 09:52 PM, Peter Hutterer wrote:
> Throw an error into the log file, but continue anyway. And after three
> warnings, stop complaining. Not all input drivers will be fixed in time (or
> ever) and our printf implementation is vastly inferior, so there is still a
> use-case for non-sigsafe logging.
>
> This also adds more linebreaks to the message.
>
> CC: Chase Douglas <chase.douglas at canonical.com>
> Signed-off-by: Peter Hutterer <peter.hutterer at who-t.net>
> ---
>   os/log.c | 39 +++++++++++++++++++++++++--------------
>   1 file changed, 25 insertions(+), 14 deletions(-)
>
> diff --git a/os/log.c b/os/log.c
> index 4348739..7fca317 100644
> --- a/os/log.c
> +++ b/os/log.c
> @@ -467,6 +467,7 @@ LogMessageTypeVerbString(MessageType type, int verb)
>   void
>   LogVMessageVerb(MessageType type, int verb, const char *format, va_list args)
>   {
> +    static unsigned int warned;
>       const char *type_str;
>       char buf[1024];
>       const size_t size = sizeof(buf);
> @@ -474,13 +475,17 @@ LogVMessageVerb(MessageType type, int verb, const char *format, va_list args)
>       size_t len = 0;
>
>       if (inSignalContext) {
> -        BUG_WARN_MSG(inSignalContext,
> -                     "Warning: attempting to log data in a signal unsafe "
> -                     "manner while in signal context. Please update to check "
> -                     "inSignalContext and/or use LogMessageVerbSigSafe() or "
> -                     "ErrorFSigSafe(). The offending log format message is:\n"
> -                     "%s\n", format);
> -        return;
> +        if (warned < 3) {
> +            BUG_WARN_MSG(inSignalContext,
> +                         "Warning: attempting to log data in a signal unsafe "
> +                         "manner while in signal context.\nPlease update to check "
> +                         "inSignalContext and/or use LogMessageVerbSigSafe() or "
> +                         "ErrorFSigSafe().\nThe offending log format message is:\n"
> +                         "%s\n", format);
> +            warned++;
> +            if (warned == 3)
> +                LogMessageVerbSigSafe(X_WARNING, -1, "Warned %u times about sigsafe logging. Will be quiet now.\n", warned);
> +        }
>       }
>
>       type_str = LogMessageTypeVerbString(type, verb);
> @@ -566,6 +571,7 @@ void
>   LogVHdrMessageVerb(MessageType type, int verb, const char *msg_format,
>                      va_list msg_args, const char *hdr_format, va_list hdr_args)
>   {
> +    static unsigned int warned;
>       const char *type_str;
>       char buf[1024];
>       const size_t size = sizeof(buf);
> @@ -573,15 +579,20 @@ LogVHdrMessageVerb(MessageType type, int verb, const char *msg_format,
>       size_t len = 0;
>
>       if (inSignalContext) {
> -        BUG_WARN_MSG(inSignalContext,
> -                     "Warning: attempting to log data in a signal unsafe "
> -                     "manner while in signal context. Please update to check "
> -                     "inSignalContext and/or use LogMessageVerbSigSafe(). The "
> -                     "offending header and log message formats are:\n%s %s\n",
> -                     hdr_format, msg_format);
> -        return;
> +        if (warned < 3) {
> +            BUG_WARN_MSG(inSignalContext,
> +                         "Warning: attempting to log data in a signal unsafe "
> +                         "manner while in signal context.\nPlease update to check "
> +                         "inSignalContext and/or use LogMessageVerbSigSafe().\nThe "
> +                         "offending header and log message formats are:\n%s %s\n",
> +                         hdr_format, msg_format);
> +            warned++;
> +            if (warned == 3)
> +                LogMessageVerbSigSafe(X_WARNING, -1, "Warned %u times about sigsafe logging. Will be quiet now.\n", warned);
> +        }
>       }
>
> +

Stray newline here ^^

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

It's too bad everything won't be fixed in time, but we've gotten along 
this far without too much issue. Hopefully the warnings will be big 
enough to force the issue to a close.

Reviewed-by: Chase Douglas <chase.douglas at canonical.com>