Re: [PATCH spice-gtk] spicy: make codec to string a bit safer

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

 



Hi

----- Original Message -----
> Hi,
> 
> On Mon, Mar 13, 2017 at 07:11:02AM -0400, Marc-André Lureau wrote:
> > Hi
> > 
> > ----- Original Message -----
> > > Hi,
> > > 
> > > On Mon, Mar 13, 2017 at 07:05:08AM -0400, Frediano Ziglio wrote:
> > > > > 
> > > > > From: Marc-André Lureau <marcandre.lureau@xxxxxxxxxx>
> > > > > 
> > > > > Handle unknown values instead of out-of-array access.
> > > > > 
> > > > > Signed-off-by: Marc-André Lureau <marcandre.lureau@xxxxxxxxxx>
> > > > > ---
> > > > >  tools/spicy.c | 28 +++++++++++++++++++---------
> > > > >  1 file changed, 19 insertions(+), 9 deletions(-)
> > > > > 
> > > > > diff --git a/tools/spicy.c b/tools/spicy.c
> > > > > index a41a1a3..2f6be4e 100644
> > > > > --- a/tools/spicy.c
> > > > > +++ b/tools/spicy.c
> > > > > @@ -181,14 +181,24 @@ static int ask_user(GtkWidget *parent, char
> > > > > *title,
> > > > > char *message,
> > > > >      return retval;
> > > > >  }
> > > > >  
> > > > > -static const gchar *video_codec_enum_to_str[] = {
> > > > > -    [0] = "none",
> > > > > -    [SPICE_VIDEO_CODEC_TYPE_MJPEG] = "mjpeg",
> > > > > -    [SPICE_VIDEO_CODEC_TYPE_VP8] = "vp8",
> > > > > -    [SPICE_VIDEO_CODEC_TYPE_H264] = "h264",
> > > > > -    [SPICE_VIDEO_CODEC_TYPE_VP9] = "vp9",
> > > > > -    [SPICE_VIDEO_CODEC_TYPE_ENUM_END] = "error",
> > > > > -};
> > > > > +static const gchar *
> > > > > +video_codec_to_string(SpiceVideoCodecType type)
> > > > > +{
> > > > > +    const char *str = NULL;
> > > > > +    static const gchar *to_string[] = {
> > > > 
> > > > this can be also const (not a regression by the way).
> > > > 
> > > > > +        NULL,
> > > > 
> > > > This should be automatic and before was "none" instead but for
> > > > me is fine.
> > > 
> > > Ah, true. None is better because 0 means that there is no ongoing
> > > stream. So I would keep that too.
> > 
> > Is it a "raw" codec then?
> 
> You mean to use 'raw' instead of 'none'?
> IMHO, "streaming: none" > "streaming: raw", but I don't mind to change
> it (but none or raw is better then "unknown codec" when value is 0)

Yes, I didn't realize [0] = 'none' was meant for a streaming codec.

I'll change it to 'raw' then.
 
> >
> > >
> > > > For SPICE_VIDEO_CODEC_TYPE_ENUM_END before an "error" was print,
> > > > now "unknown codec". Again, fo me is fine.
> > > >
> > > > > +        [SPICE_VIDEO_CODEC_TYPE_MJPEG] = "mjpeg",
> > > > > +        [SPICE_VIDEO_CODEC_TYPE_VP8] = "vp8",
> > > > > +        [SPICE_VIDEO_CODEC_TYPE_H264] = "h264",
> > > > > +        [SPICE_VIDEO_CODEC_TYPE_VP9] = "vp9",
> > > > > +    };
> > > > > +
> > > > > +    if (type >= 0 && type < G_N_ELEMENTS(to_string)) {
> > > > > +        str = to_string[type];
> > > > > +    }
> > > > > +
> > > > > +    return str ? str : "unknown codec";
> > > > > +}
> > > > >  
> > > > >  static void update_status_window(SpiceWindow *win)
> > > > >  {
> > > > > @@ -201,7 +211,7 @@ static void update_status_window(SpiceWindow
> > > > > *win)
> > > > >      g_string_printf(status, "mouse: %6s, agent: %3s, streaming:
> > > > >      %5s",
> > > > >                      win->conn->mouse_state,
> > > > >                      win->conn->agent_state,
> > > > > -                    video_codec_enum_to_str[win->video_codec]);
> > > > > +                    video_codec_to_string(win->video_codec));
> > > > >  
> > > > >      if (win->mouse_grabbed) {
> > > > >          SpiceGrabSequence *sequence =
> > > > >          spice_display_get_grab_keys(SPICE_DISPLAY(win->spice));
> > > > 
> > > > Reviewed-by: Frediano Ziglio <fziglio@xxxxxxxxxx>
> > > > 
> > > > Frediano
> > > > _______________________________________________
> > > > Spice-devel mailing list
> > > > Spice-devel@xxxxxxxxxxxxxxxxxxxxx
> > > > https://lists.freedesktop.org/mailman/listinfo/spice-devel
> > > 
> 
_______________________________________________
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]