Re: [PATCH 02/18] Print warnings on untested code paths

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, 2016-04-28 at 13:02 -0400, Frediano Ziglio wrote:
> > 
> > ---
> >  server/display-channel.c | 3 +++
> >  server/sound.c           | 1 +
> >  2 files changed, 4 insertions(+)
> > 
> > diff --git a/server/display-channel.c b/server/display-channel.c
> > index 1f4d66f..3a06305 100644
> > --- a/server/display-channel.c
> > +++ b/server/display-channel.c
> > @@ -1921,6 +1921,9 @@ void display_channel_create_surface(DisplayChannel
> > *display, uint32_t surface_id
> >          QXLInstance *qxl = display->common.qxl;
> >          RedsState *reds = red_qxl_get_server(qxl->st);
> >          GArray *renderers = reds_get_renderers(reds);
> > +        /* These days, noone is trying to use multiple renderers, the
> > software one
> > +         * is always used */
> > +        g_warn_if_fail(renderers->len == 1);
> >          for (i = 0; i < renderers->len; i++) {
> >              uint32_t renderer = g_array_index(renderers, uint32_t, i);
> >              surface->context.canvas = create_canvas_for_surface(display,
> >              surface, renderer);
> > diff --git a/server/sound.c b/server/sound.c
> > index aae841c..b95e7e7 100644
> > --- a/server/sound.c
> > +++ b/server/sound.c
> > @@ -1620,6 +1620,7 @@ void snd_set_playback_compression(int on)
> >      SndWorker *now = workers;
> >  
> >      for (; now; now = now->next) {
> > +        g_critical("untested code path");
> >          if (now->base_channel->type == SPICE_CHANNEL_PLAYBACK &&
> >          now->connection) {
> >              PlaybackChannel* playback = (PlaybackChannel*)now->connection;
> >              SpicePlaybackState *st = SPICE_CONTAINEROF(now,
> >              SpicePlaybackState, worker);
> 
> I would prefer a better option for this last.
> The function is not currently called by Qemu.

That's not quite true. it is called from qemu_spice_init(). When called at this
point, no sound workers have been created yet, so the code within the loop does
not get executed. So the only effect of qemu calling the function is to set the
value of reds->config->streaming_video, which is then later used when the sound
workers are actually created.

> We could deprecate and make it do nothing. It's just disabling the sound
> compression (which is on by default) and anyway the compression are also
> handled by capabilities.
> Even better would be writing a test for this function.

Yes, it should really have a test. My suggestion would be to move this patch way
to the end of the branch and deal with it later after all other patches have
been merged.

> 
> Frediano
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel




[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]     [Monitors]