>
> On Fri, 2016-10-28 at 06:46 -0400, Frediano Ziglio wrote:
> > >
> > >
> > > In commit beec1b41, we manually limited this property value in
> > > _set_property(). But there's a simpler way to do it: via the param
> > > spec
> > > for the property.
> > >
> > > This also means that we can remove the warning log in
> > > red_worker_new()
> > > since GObject will automatically warn if a property is assigned a
> > > value
> > > outside of its valid range.
> > > ---
> > >
> > > I know the earlier commit was already pushed, but I just realized a
> > > simpler
> > > way
> > > to fix this issue. What do you think?
> > >
> > > server/display-channel.c | 3 +--
> > > server/red-worker.c | 2 --
> > > 2 files changed, 1 insertion(+), 4 deletions(-)
> > >
> > > diff --git a/server/display-channel.c b/server/display-channel.c
> > > index bcf26bb..b838ba6 100644
> > > --- a/server/display-channel.c
> > > +++ b/server/display-channel.c
> > > @@ -60,7 +60,6 @@ display_channel_set_property(GObject *object,
> > > {
> > > case PROP_N_SURFACES:
> > > self->priv->n_surfaces = g_value_get_uint(value);
> > > - self->priv->n_surfaces = MIN(self->priv->n_surfaces,
> > > NUM_SURFACES);
> > > break;
> > > case PROP_VIDEO_CODECS:
> > > if (self->priv->video_codecs) {
> > > @@ -2232,7 +2231,7 @@
> > > display_channel_class_init(DisplayChannelClass *klass)
> > > g_param_spec_uint("n-
> > > surfaces",
> > > "number of
> > > surfaces",
> > > "Number of
> > > surfaces
> > > for this
> > > channel",
> > > - 0,
> > > G_MAXUINT,
> > > + 0,
> > > NUM_SURFACES,
> > > 0,
> > > G_PARAM_CONS
> > > TRUCT_ONLY
> > > |
> > > G_PARAM_READ
> > > WRITE |
> > > diff --git a/server/red-worker.c b/server/red-worker.c
> > > index f7f0726..2ed2453 100644
> > > --- a/server/red-worker.c
> > > +++ b/server/red-worker.c
> > > @@ -1358,8 +1358,6 @@ RedWorker* red_worker_new(QXLInstance *qxl,
> > > init_info.memslot_id_bits,
> > > init_info.internal_groupslot_id);
> > >
> > > - spice_warn_if_fail(init_info.n_surfaces <= NUM_SURFACES);
> > > -
> > > worker->event_timeout = INF_EVENT_WAIT;
> > >
> > > worker->cursor_channel = cursor_channel_new(reds, qxl,
> >
> > Sounds good. What happens to the property? Does the value
> > get capped or just discarded?
>
> The invalid value is discarded and a critical warning is issued.
>
> > Not that Qemu should try to set crazy values but if you set 20000
> > and got 0 perhaps would be better to get the maximum (10000).
> > Surely better than giving a warning and then having possible
> > overflows (as the current code does!).
>
> Well, at the moment, we set the default value for that parameter to 0.
> But we could also change the default to make it something more
> reasonable (perhaps the maximum?). Would that be sufficient, or would
> you still prefer to keep your existing "capped" approach?
>
> Jonathon
>
Would be fine.
About ranges I think the minimum should be 1 so to allow to
have a primary surface (always 0!).
Frediano
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel