Re: [PATCH spice-server 1/2] Change ENABLE_EXTRA_CHECKS statements to #ifdef

[Uri Lublin <uril@xxxxxxxxxx>]

On 03/13/2018 03:37 PM, Eduardo Lima (Etrunko) wrote:
> On 13/03/18 04:21, Frediano Ziglio wrote:
> > > This patch makes it clear that this is a configure switch and not a
> > > variable defined somewhere else in the code.
> >
> > The code is intended that way to make the compiler always parse
> > these parts. Note that that define is always defined so your code
> > is not doing what you are intending.
>
> I have sent this patch by mistake, but anyway, the fact of it always
> being defined is true with autotools, but it is not with meson.

So with meson it's either 1 or undefined ?

Note that in this case the current code would fail to build
when ENABLE_EXTRA_CHECKS is undefined.

> Do you think it would make sense to have this patch or is it better to
> keep as is? If the latter, I think it would be better to keep a static
> variable and change its value according to the define.

This patch makes sense to me.

Perhaps replace the #ifdef with
#if defined(ENABLE_EXTRA_CHECKS) && ENABLE_EXTRA_CHECKS

Uri.

> Regards, Eduardo.
>
> > Frediano
> >
> > > Signed-off-by: Eduardo Lima (Etrunko) <etrunko@xxxxxxxxxx>
> > > ---
> > >   server/display-channel.c |  4 +++-
> > >   server/reds.c            |  6 +++---
> > >   server/stream-device.c   | 22 +++++++++++-----------
> > >   3 files changed, 17 insertions(+), 15 deletions(-)
> > >
> > > diff --git a/server/display-channel.c b/server/display-channel.c
> > > index 6dc10ee7..ae2af71d 100644
> > > --- a/server/display-channel.c
> > > +++ b/server/display-channel.c
> > > @@ -89,7 +89,8 @@ display_channel_finalize(GObject *object)
> > >       display_channel_destroy_surfaces(self);
> > >       image_cache_reset(&self->priv->image_cache);
> > >   
> > > -    if (ENABLE_EXTRA_CHECKS) {
> > > +#ifdef ENABLE_EXTRA_CHECKS
> > > +    {
> > >           unsigned int count;
> > >           _Drawable *drawable;
> > >           VideoStream *stream;
> > > @@ -111,6 +112,7 @@ display_channel_finalize(GObject *object)
> > >               spice_assert(self->priv->surfaces[count].context.canvas ==
> > >               NULL);
> > >           }
> > >       }
> > > +#endif
> > >   
> > >       monitors_config_unref(self->priv->monitors_config);
> > >       g_array_unref(self->priv->video_codecs);
> > > diff --git a/server/reds.c b/server/reds.c
> > > index 73c9ec20..ca6fd44a 100644
> > > --- a/server/reds.c
> > > +++ b/server/reds.c
> > > @@ -4434,9 +4434,9 @@ red_char_device_vdi_port_finalize(GObject *object)
> > >           dev->priv->current_read_buf = NULL;
> > >       }
> > >       g_free(dev->priv->mig_data);
> > > -    if (ENABLE_EXTRA_CHECKS) {
> > > -        spice_assert(dev->priv->num_read_buf == 0);
> > > -    }
> > > +#ifdef ENABLE_EXTRA_CHECKS
> > > +    spice_assert(dev->priv->num_read_buf == 0);
> > > +#endif
> > >   
> > >       G_OBJECT_CLASS(red_char_device_vdi_port_parent_class)->finalize(object);
> > >   }
> > > diff --git a/server/stream-device.c b/server/stream-device.c
> > > index 6cf29d37..15048b82 100644
> > > --- a/server/stream-device.c
> > > +++ b/server/stream-device.c
> > > @@ -197,9 +197,9 @@ handle_msg_invalid(StreamDevice *dev,
> > > SpiceCharDeviceInstance *sin, const char *
> > >   {
> > >       static const char default_error_msg[] = "Protocol error";
> > >   
> > > -    if (ENABLE_EXTRA_CHECKS) {
> > > -        spice_assert(dev->hdr_pos >= sizeof(StreamDevHeader));
> > > -    }
> > > +#ifdef ENABLE_EXTRA_CHECKS
> > > +    spice_assert(dev->hdr_pos >= sizeof(StreamDevHeader));
> > > +#endif
> > >   
> > >       if (!error_msg) {
> > >           error_msg = default_error_msg;
> > > @@ -234,10 +234,10 @@ handle_msg_format(StreamDevice *dev,
> > > SpiceCharDeviceInstance *sin)
> > >   {
> > >       SpiceCharDeviceInterface *sif = spice_char_device_get_interface(sin);
> > >   
> > > -    if (ENABLE_EXTRA_CHECKS) {
> > > -        spice_assert(dev->hdr_pos >= sizeof(StreamDevHeader));
> > > -        spice_assert(dev->hdr.type == STREAM_TYPE_FORMAT);
> > > -    }
> > > +#ifdef ENABLE_EXTRA_CHECKS
> > > +    spice_assert(dev->hdr_pos >= sizeof(StreamDevHeader));
> > > +    spice_assert(dev->hdr.type == STREAM_TYPE_FORMAT);
> > > +#endif
> > >   
> > >       int n = sif->read(sin, dev->msg->buf + dev->msg_pos,
> > >       sizeof(StreamMsgFormat) - dev->msg_pos);
> > >       if (n < 0) {
> > > @@ -262,10 +262,10 @@ handle_msg_data(StreamDevice *dev,
> > > SpiceCharDeviceInstance *sin)
> > >       SpiceCharDeviceInterface *sif = spice_char_device_get_interface(sin);
> > >       int n;
> > >   
> > > -    if (ENABLE_EXTRA_CHECKS) {
> > > -        spice_assert(dev->hdr_pos >= sizeof(StreamDevHeader));
> > > -        spice_assert(dev->hdr.type == STREAM_TYPE_DATA);
> > > -    }
> > > +#ifdef ENABLE_EXTRA_CHECKS
> > > +    spice_assert(dev->hdr_pos >= sizeof(StreamDevHeader));
> > > +    spice_assert(dev->hdr.type == STREAM_TYPE_DATA);
> > > +#endif
> > >   
> > >       while (1) {
> > >           uint8_t buf[16 * 1024];

_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel