[Spice-devel] [PATCH spice-server 1/2] Change ENABLE_EXTRA_CHECKS statements to #ifdef

[Uri Lublin]

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 at redhat.com>
>>> ---
>>>   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];
> 
>