future proofing GConf for backend vtable changes

[Mark McLoughlin <mark skynet ie>]

Hi Cyrille,
	In your patch in bug #107692 (allowing notifications from the backend)
you added a new notifier vtable rather than extending the existing
vtable because you were worried about breaking compatibility with
existing backends.

	Since the backend API is completely private to GConf right now, we
don't really need to worry about this but I thought it might be nice to
add some future proofing before extending the vtable any further.

	The attached patch takes the following approach:

  + The backends are compiled with the vtable's size as the first member
    of the structure.

  + When initializing a backend, we create a new vtable instance 
    according to our knowledge of its size and fill it with whatever the
    backend provided us.

  + This means that if the backend was compiled with an older version of
    the vtable, the new members are guaranteed to be initialized to NULL
    and if it was compiled with a new version of the vtable than the 
    library, then we just ignore any new methods.

	There's some other stuff in the patch too - this part is the only
really relevant bit:

  retval = g_new0(GConfBackendVTable, 1);

  retval = memcpy(retval, vtable, MIN(vtable->vtable_size, sizeof(GConfBackendVTable)));

	So, how does that sound? Havoc?

Cheers,
Mark.

Index: backends/markup-backend.c
===================================================================
RCS file: /cvs/gnome/gconf/backends/markup-backend.c,v
retrieving revision 1.7
diff -u -p -r1.7 markup-backend.c
--- backends/markup-backend.c	27 Oct 2003 18:11:17 -0000	1.7
+++ backends/markup-backend.c	23 Mar 2004 21:22:37 -0000
@@ -132,6 +132,7 @@ static void           blow_away_locks (c
 
 
 static GConfBackendVTable markup_vtable = {
+  sizeof (GConfBackendVTable),
   x_shutdown,
   resolve_address,
   lock,
Index: backends/xml-backend.c
===================================================================
RCS file: /cvs/gnome/gconf/backends/xml-backend.c,v
retrieving revision 1.85
diff -u -p -r1.85 xml-backend.c
--- backends/xml-backend.c	22 Oct 2003 21:03:04 -0000	1.85
+++ backends/xml-backend.c	23 Mar 2004 21:22:37 -0000
@@ -209,6 +209,7 @@ static void          clear_cache     (GC
 static void          blow_away_locks (const char *address);
 
 static GConfBackendVTable xml_vtable = {
+  sizeof (GConfBackendVTable),
   x_shutdown,
   resolve_address,
   lock,
Index: gconf/gconf-backend.c
===================================================================
RCS file: /cvs/gnome/gconf/gconf/gconf-backend.c,v
retrieving revision 1.22
diff -u -p -r1.22 gconf-backend.c
--- gconf/gconf-backend.c	27 Mar 2003 01:41:14 -0000	1.22
+++ gconf/gconf-backend.c	23 Mar 2004 21:22:38 -0000
@@ -178,6 +178,63 @@ gconf_backend_file(const gchar* address)
 
 static GHashTable* loaded_backends = NULL;
 
+static GConfBackendVTable *
+gconf_backend_check_vtable (GConfBackendVTable  *vtable,
+			    const char          *backend_name,			    
+			    GError             **err)
+{
+  GConfBackendVTable *retval;
+  int i;
+  struct
+  {
+    char  *name;
+    gsize  offset;
+  } required_vtable_functions[] = {
+    { "shutdown",        G_STRUCT_OFFSET(GConfBackendVTable, shutdown)        },
+    { "resolve_address", G_STRUCT_OFFSET(GConfBackendVTable, resolve_address) },
+    { "query_value",     G_STRUCT_OFFSET(GConfBackendVTable, query_value)     },
+    { "query_metainfo",  G_STRUCT_OFFSET(GConfBackendVTable, query_metainfo)  },
+    { "set_value",       G_STRUCT_OFFSET(GConfBackendVTable, set_value)       },
+    { "all_entries",     G_STRUCT_OFFSET(GConfBackendVTable, all_entries)     },
+    { "all_subdirs",     G_STRUCT_OFFSET(GConfBackendVTable, all_subdirs)     },
+    { "unset_value",     G_STRUCT_OFFSET(GConfBackendVTable, unset_value)     },
+    { "dir_exists",      G_STRUCT_OFFSET(GConfBackendVTable, dir_exists)      },
+    { "remove_dir",      G_STRUCT_OFFSET(GConfBackendVTable, remove_dir)      },
+    { "set_schema",      G_STRUCT_OFFSET(GConfBackendVTable, set_schema)      },
+    { "sync_all",        G_STRUCT_OFFSET(GConfBackendVTable, sync_all)        },
+    { "destroy_source",  G_STRUCT_OFFSET(GConfBackendVTable, destroy_source)  },
+    { "blow_away_locks", G_STRUCT_OFFSET(GConfBackendVTable, blow_away_locks) }
+  };
+
+  if (!vtable)
+    {
+      gconf_set_error(err,
+		      GCONF_ERROR_FAILED, _("Backend `%s' failed return a vtable\n"),
+		      backend_name);
+      return NULL;
+    }
+
+  /* Create a copy in case vtable size doesn't match */
+  retval = g_new0(GConfBackendVTable, 1);
+
+  retval = memcpy(retval, vtable, MIN(vtable->vtable_size, sizeof(GConfBackendVTable)));
+
+  for (i = 0; i < G_N_ELEMENTS(required_vtable_functions); i++)
+    {
+      if (G_STRUCT_MEMBER_P(retval, required_vtable_functions[i].offset) == NULL)
+	{
+	  gconf_set_error(err,
+			  GCONF_ERROR_FAILED, _("Backend `%s' missing required vtable member `%s'\n"),
+			  backend_name,
+			  required_vtable_functions[i].name);
+	  g_free(retval);
+	  return NULL;
+	}
+    }
+
+  return retval;
+}
+
 GConfBackend* 
 gconf_get_backend(const gchar* address, GError** err)
 {
@@ -225,10 +282,9 @@ gconf_get_backend(const gchar* address, 
           
           if (module == NULL)
             {
-              const gchar* error = g_module_error();
               gconf_set_error(err,
                               GCONF_ERROR_FAILED, _("Error opening module `%s': %s\n"),
-                              name, error);
+                              name, g_module_error());
               g_free(name);
               return NULL;
             }
@@ -237,6 +293,10 @@ gconf_get_backend(const gchar* address, 
                                "gconf_backend_get_vtable", 
                                (gpointer*)&get_vtable))
             {
+              gconf_set_error(err,
+                              GCONF_ERROR_FAILED, _("Error initializing module `%s': %s\n"),
+                              name, g_module_error());
+              g_module_close(module);
               g_free(name);
               return NULL;
             }
@@ -245,7 +305,14 @@ gconf_get_backend(const gchar* address, 
 
           backend->module = module;
 
-          backend->vtable = (*get_vtable)();
+	  backend->vtable = gconf_backend_check_vtable((*get_vtable)(), name, err);
+	  if (!backend->vtable)
+	    {
+	      g_module_close(module);
+	      g_free(name);
+	      g_free(backend);
+	      return NULL;
+	    }
               
           backend->name = name;
 
@@ -300,6 +367,7 @@ gconf_backend_unref(GConfBackend* backen
 
       g_hash_table_remove(loaded_backends, backend->name);
       
+      g_free(backend->vtable);
       g_free((gchar*)backend->name); /* cast off const */
 
       g_free(backend);
Index: gconf/gconf-backend.h
===================================================================
RCS file: /cvs/gnome/gconf/gconf/gconf-backend.h,v
retrieving revision 1.30
diff -u -p -r1.30 gconf-backend.h
--- gconf/gconf-backend.h	27 Mar 2003 01:41:14 -0000	1.30
+++ gconf/gconf-backend.h	23 Mar 2004 21:22:38 -0000
@@ -33,6 +33,9 @@
 typedef struct _GConfBackendVTable GConfBackendVTable;
 
 struct _GConfBackendVTable {
+  /* Set to sizeof (GConfBackendVTable) - used for future proofing */
+  gsize                  vtable_size;
+
   void                (* shutdown)        (GError** err);
 
   GConfSource*        (* resolve_address) (const gchar* address,