[Spice-devel] [PATCH] Limit maximum "n-surfaces" via param spec

[Frediano Ziglio]

> 
> 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_CONSTRUCT_ONLY
>                                                        |
>                                                        G_PARAM_READWRITE |
> 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?
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!).

Frediano