[Spice-devel] [protocol 0/3] Fixing the *_DEPRECATED set of macros

Francois Gouget fgouget at codeweavers.com
Mon Oct 17 12:36:48 UTC 2016


On Thu, 1 Sep 2016, Christophe Fergeau wrote:

> On Thu, Aug 11, 2016 at 04:28:58PM +0200, Francois Gouget wrote:
> > 
> > The following patch broke compilation of xf86-video-qxl:
> > 
> > commit b41220b1441b8adea6db9a98e9da1b43a8f426bb
> > Author: Christophe Fergeau <cfergeau at redhat.com>
> > Date:   Thu Mar 5 15:28:22 2015 +0100
> > 
> >     Mark unused public API methods/code as deprecated
> >     
> >     Acked-by: Jonathon Jongsma <jjongsma at redhat.com>
> > 
> > 
> > The reason is it introduces a #include <glib.h> in spice-server.h which 
> > is a *public* header! This means any application that needs to include 
> > spice-server.h, like xf86-video-qxl, must now check for GLib even if 
> > they don't use it, like xf86-video-qxl.
> > 
> > This does not make sense and thus the glib.h include must be removed. 
> 
> As danpb pointed out, main reason for that is that we forgot to add
> glib-2.0 as a dependency in the .pc file, if this had been done, this
> change would probably have gone mostly unnoticed.

No. The main reason is that the patch introduced a dependency which 
should not be there. Papering over it with a .pc file changes nothing to 
the fact that third-party projects will now be unable to use the header 
without installing glib first and that there is no good reason for that.



[...]
> Imo this SPICE_NO_DEPRECATED is only meant to be used internally by
> spice-gtk even if this is not really documented. I would not consider it
> breakage if this was removed, or changed in incompatible ways.
> And it turns out it's actually useless as -Wno-deprecated-declarations
> is used during compilation (which also disables the
> warnings from GLIB_VERSION_MIN_REQUIRED/GLIB_VERSION_MAX_REQUIRED :(
> I'd tend to change that so that -Wno-deprecated-declarations is only
> used for spicy, and make selective use of
> G_GNUC_BEGIN_IGNORE_DEPRECATIONS / G_GNUC_END_IGNORE_DEPRECATIONS when
> needed. Seems to be working with quick and dirty local changes.

I know you were talking about spice-gtk, but would the spice patch 
below be what you had in mind?


commit 8b6bb2357ba41426d08c0f322440c19cb8e1b897
Author: Francois Gouget <fgouget at codeweavers.com>
Date:   Mon Oct 17 11:48:47 2016 +0200

    server: Disable deprecation warnings only where needed
    
    Signed-off-by: Francois Gouget <fgouget at codeweavers.com>

diff --git a/server/red-qxl.c b/server/red-qxl.c
index e517b41..51e0dd6 100644
--- a/server/red-qxl.c
+++ b/server/red-qxl.c
@@ -971,6 +971,7 @@ void red_qxl_init(RedsState *reds, QXLInstance *qxl)
     qxl_state->dispatcher = dispatcher_new(RED_WORKER_MESSAGE_COUNT, NULL);
     qxl_state->qxl_worker.major_version = SPICE_INTERFACE_QXL_MAJOR;
     qxl_state->qxl_worker.minor_version = SPICE_INTERFACE_QXL_MINOR;
+    G_GNUC_BEGIN_IGNORE_DEPRECATIONS
     qxl_state->qxl_worker.wakeup = qxl_worker_wakeup;
     qxl_state->qxl_worker.oom = qxl_worker_oom;
     qxl_state->qxl_worker.start = qxl_worker_start;
@@ -987,6 +988,7 @@ void red_qxl_init(RedsState *reds, QXLInstance *qxl)
     qxl_state->qxl_worker.reset_cursor = qxl_worker_reset_cursor;
     qxl_state->qxl_worker.destroy_surface_wait = qxl_worker_destroy_surface_wait;
     qxl_state->qxl_worker.loadvm_commands = qxl_worker_loadvm_commands;
+    G_GNUC_END_IGNORE_DEPRECATIONS
 
     qxl_state->max_monitors = UINT_MAX;
     qxl->st = qxl_state;
diff --git a/server/red-replay-qxl.c b/server/red-replay-qxl.c
index b5baded..5dc173e 100644
--- a/server/red-replay-qxl.c
+++ b/server/red-replay-qxl.c
@@ -1241,7 +1241,9 @@ static void replay_handle_create_primary(QXLWorker *worker, SpiceReplay *replay)
         spice_printerr(
             "WARNING: %d: original recording event not preceded by a destroy primary",
             replay->counter);
+        G_GNUC_BEGIN_IGNORE_DEPRECATIONS
         worker->destroy_primary_surface(worker, 0);
+        G_GNUC_END_IGNORE_DEPRECATIONS
     }
     replay->created_primary = TRUE;
 
@@ -1255,7 +1257,9 @@ static void replay_handle_create_primary(QXLWorker *worker, SpiceReplay *replay)
     read_binary(replay, "data", &size, &mem, 0);
     surface.group_id = 0;
     surface.mem = QXLPHYSICAL_FROM_PTR(mem);
+    G_GNUC_BEGIN_IGNORE_DEPRECATIONS
     worker->create_primary_surface(worker, 0, &surface);
+    G_GNUC_END_IGNORE_DEPRECATIONS
 }
 
 static void replay_handle_dev_input(QXLWorker *worker, SpiceReplay *replay,
@@ -1264,15 +1268,21 @@ static void replay_handle_dev_input(QXLWorker *worker, SpiceReplay *replay,
     switch (message) {
     case RED_WORKER_MESSAGE_CREATE_PRIMARY_SURFACE:
     case RED_WORKER_MESSAGE_CREATE_PRIMARY_SURFACE_ASYNC:
+        G_GNUC_BEGIN_IGNORE_DEPRECATIONS
         replay_handle_create_primary(worker, replay);
+        G_GNUC_END_IGNORE_DEPRECATIONS
         break;
     case RED_WORKER_MESSAGE_DESTROY_PRIMARY_SURFACE:
         replay->created_primary = FALSE;
+        G_GNUC_BEGIN_IGNORE_DEPRECATIONS
         worker->destroy_primary_surface(worker, 0);
+        G_GNUC_END_IGNORE_DEPRECATIONS
         break;
     case RED_WORKER_MESSAGE_DESTROY_SURFACES:
         replay->created_primary = FALSE;
+        G_GNUC_BEGIN_IGNORE_DEPRECATIONS
         worker->destroy_surfaces(worker);
+        G_GNUC_END_IGNORE_DEPRECATIONS
         break;
     case RED_WORKER_MESSAGE_UPDATE:
         // XXX do anything? we record the correct bitmaps already.
diff --git a/server/reds-stream.c b/server/reds-stream.c
index 9896eab..1f95b13 100644
--- a/server/reds-stream.c
+++ b/server/reds-stream.c
@@ -358,10 +358,12 @@ static void reds_stream_set_socket(RedsStream *stream, int socket)
 {
     stream->socket = socket;
     /* deprecated fields. Filling them for backward compatibility */
+    G_GNUC_BEGIN_IGNORE_DEPRECATIONS
     stream->priv->info->llen = sizeof(stream->priv->info->laddr);
     stream->priv->info->plen = sizeof(stream->priv->info->paddr);
     getsockname(stream->socket, (struct sockaddr*)(&stream->priv->info->laddr), &stream->priv->info->llen);
     getpeername(stream->socket, (struct sockaddr*)(&stream->priv->info->paddr), &stream->priv->info->plen);
+    G_GNUC_END_IGNORE_DEPRECATIONS
 
     stream->priv->info->flags |= SPICE_CHANNEL_EVENT_FLAG_ADDR_EXT;
     stream->priv->info->llen_ext = sizeof(stream->priv->info->laddr_ext);
diff --git a/server/spice-core.h b/server/spice-core.h
index 4d8f2ed..dfd24ab 100644
--- a/server/spice-core.h
+++ b/server/spice-core.h
@@ -28,10 +28,6 @@
 #include <spice/vd_agent.h>
 #include <spice/macros.h>
 
-#ifdef SPICE_SERVER_INTERNAL
-#undef SPICE_GNUC_DEPRECATED
-#define SPICE_GNUC_DEPRECATED
-#endif
 
 /* interface base type */
 

-- 
Francois Gouget <fgouget at codeweavers.com>


More information about the Spice-devel mailing list