[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] hw/audio/sb16.c: Convert file to new logging AP
From: |
Programmingkid |
Subject: |
Re: [Qemu-devel] [PATCH] hw/audio/sb16.c: Convert file to new logging API |
Date: |
Wed, 31 Jan 2018 16:43:53 -0500 |
> On Jan 31, 2018, at 4:22 AM, Gerd Hoffmann <address@hidden> wrote:
>
> On Tue, Jan 30, 2018 at 10:30:46AM -0500, Programmingkid wrote:
>>
>>> On Jan 30, 2018, at 4:41 AM, Gerd Hoffmann <address@hidden> wrote:
>>>
>>>> @@ -148,15 +142,16 @@ static int irq_of_magic (int magic)
>>>> #if 0
>>>> static void log_dsp (SB16State *dsp)
>>>> {
>>>> - ldebug ("%s:%s:%d:%s:dmasize=%d:freq=%d:const=%d:speaker=%d\n",
>>>> - dsp->fmt_stereo ? "Stereo" : "Mono",
>>>> - dsp->fmt_signed ? "Signed" : "Unsigned",
>>>> - dsp->fmt_bits,
>>>> - dsp->dma_auto ? "Auto" : "Single",
>>>> - dsp->block_size,
>>>> - dsp->freq,
>>>> - dsp->time_const,
>>>> - dsp->speaker);
>>>> + qemu_log_mask(LOG_UNIMP, "%s:%s:%d:%s:dmasize=%d:freq=%d:const=%d:"
>>>> + "speaker=%d\n",
>>>> + dsp->fmt_stereo ? "Stereo" : "Mono",
>>>> + dsp->fmt_signed ? "Signed" : "Unsigned",
>>>> + dsp->fmt_bits,
>>>> + dsp->dma_auto ? "Auto" : "Single",
>>>> + dsp->block_size,
>>>> + dsp->freq,
>>>> + dsp->time_const,
>>>> + dsp->speaker);
>>>> }
>>>> #endif
>>>
>>> Hmm, dead code. Any places which call log_dsp() ?
>>
>> There are several places but they are all dead code. Do you want this
>> removed?
>
> So it is still used but not compiled in by default.
>
> Looking at all of this again, I think "qemu_log_mask(LOG_UNIMP, ...)"
> should only be used in case there *really* is some soundblaster feature
> unimplemented. Otherwise converting to a tracepoint is probably more
> useful, so the debug messages can be enabled/disabled individually at
> runtime.
>
> The "#if 0" seems to be there to limit the flood of debug messages. So
> once they all are converted to trace points this is not needed any more
> and I think we should just enable all this code.
>
> dolog() seems to be mostly cases where the code warns about
> unimplemented featues or incomplete emulation, so using LOG_UNIMP looks
> sensible.
Simply replacing dolog() with "qemu_log_mask(LOG_UNIMP," seems like what was
desired.
> ldebug() should probably trace points.
I need help with this. I tried replacing ldebug() with
"qemu_log_mask(LOG_TRACE,", but that caused a lot of messages to be printed to
the terminal. It is odd because I thought I had to use "-d trace" to make it
print. Do you have another suggestion on what to replace ldebug() with?
> Probably makes sense to split this patch up into a small series.
The first patch will be the replacing dolog() with qemu_log_mask(LOG_UNIMP. The
second patch will be replacing ldebug() with what you suggest.