>
> 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.
>
So you should document that this function is designed to be
used just by spice_server_set_channel_security.
>
> >
> > > + [ 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
>
If this is the design of the function this should be documented (this
function is designed to be called only by red_channel_get_name).
Frediano
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel