Re: [Qemu-devel] [PATCH] hw/audio/sb16.c: Convert file to new logging API

[Gerd Hoffmann]

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.

ldebug() should probably trace points.

Probably makes sense to split this patch up into a small series.

cheers,
  Gerd