On 15/03/18 08:19, Uri Lublin wrote:
> 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 ?
Well, sort of, this can worked around of course, to have it defined to
either 1 or 0.
>
> Note that in this case the current code would fail to build
> when ENABLE_EXTRA_CHECKS is undefined.
>
That's exactly what happened, I got a build failure and then I started
to think of a better solution, which I proposed as a global static variable.
>> 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];
>>
>>
>
--
Eduardo de Barros Lima (Etrunko)
Software Engineer - RedHat
etrunko@xxxxxxxxxx
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel