Re: [RFC PATCH] logging: add a LOG_TCG_WARN for temp leaks

[Alex Bennée]

Peter Maydell <peter.maydell@linaro.org> writes:

> On Thu, 11 Jun 2020 at 15:45, Alex Bennée <alex.bennee@linaro.org> wrote:
>>
>> Pretty much all calls to qemu_log are either wrapped in some other
>> enabling check or only enabled with debug defines. Add a specific flag
>> for TCG warnings and expand the documentation of the qemu_log
>> function.
>>
>> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>> Cc: Peter Maydell <peter.maydell@linaro.org>
>> ---
>
>> -/* main logging function */
>> +/**
>> + * qemu_log: main logging function
>> + *
>> + * Most users shouldn't be calling qemu_log unconditionally as it adds
>> + * noise to logging output. Either use qemu_log_mask() or wrap
>> + * successive log calls a qemu_loglevel_mask() check and
> "inside a"
>
>> + * qemu_log_lock/unlock(). The tracing infrastructure does similar wrapping.
>> + */
>>  int GCC_FMT_ATTR(1, 2) qemu_log(const char *fmt, ...);
>
>> +/* Additional TCG warnings */
>> +#define LOG_TCG_WARN       (1 << 20)
>
> I don't object to the new log group in principle, but it has exactly
> one warning in it. I feel we'd be better to check for all the current
> places that use qemu_log not inside a loglevel_mask condition (or
> which use fprintf, if we still have those) and then see what the
> most reasonable categorization is.

I did a grep of qemu_log and fprintf cases in accel/tcg and tcg and this
was the only one that wasn't:

  - either wrapped by qemu_loglevel_mask()
  - part of an abort/exit() path (which should arguably be converted to 
error_report/abort)

In the wider code most of the the qemu_logs() I found where in D()
functions in the various device emulations.

-- 
Alex Bennée