Re: [spice-server 1/5] utils: Introduce helpers to map channel types to names

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

 



On Tue, Oct 17, 2017 at 05:15:22PM -0400, Frediano Ziglio wrote:
> > +static const char *const channel_names[] = {
> > +    [ SPICE_CHANNEL_MAIN     ] = "main",
> > +    [ SPICE_CHANNEL_DISPLAY  ] = "display",
> > +    [ SPICE_CHANNEL_INPUTS   ] = "inputs",
> > +    [ SPICE_CHANNEL_CURSOR   ] = "cursor",
> > +    [ SPICE_CHANNEL_PLAYBACK ] = "playback",
> > +    [ SPICE_CHANNEL_RECORD   ] = "record",
> > +#ifdef USE_SMARTCARD
> > +    [ SPICE_CHANNEL_SMARTCARD] = "smartcard",
> > +#endif
> 
> Not a regression. Is it worth having the #ifdef ?
> SPICE_CHANNEL_SMARTCARD is always defined.

Hmm, red_channel_name_to_type() uses that to return -1 when we were
built without smartcard support. I think it's better if we keep that
behaviour.


> 
> > +    [ SPICE_CHANNEL_USBREDIR ] = "usbredir",
> > +    [ SPICE_CHANNEL_WEBDAV ] = "webdav",
> > +};
> > +
> > +const char *red_channel_type_to_str(int type)
> > +{
> > +    g_return_val_if_fail(type >= 0, NULL);
> > +    g_return_val_if_fail(type < G_N_ELEMENTS(channel_names), NULL);
> 
> Do we want to log these? As far as I know these macro does logging.
> Currently looks like a regression.

In my opinion, red_channel_type_to_str() should not be called with an
invalid type, the caller should make sure this does not happen. At the
moment, the only caller is red_channel_get_name() which is added in this
commit as well, so this can't be a regression.

Christophe

Attachment: signature.asc
Description: PGP signature

_______________________________________________
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]