[Bug 23920] Should use GResolver instead of /GibberResolver

bugzilla-daemon at freedesktop.org bugzilla-daemon at freedesktop.org
Mon Nov 30 14:04:48 CET 2009


http://bugs.freedesktop.org/show_bug.cgi?id=23920


Sjoerd Simons <sjoerd at luon.net> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |sjoerd at luon.net
         AssignedTo|telepathy-                  |cmaiku at gmail.com
                   |bugs at lists.freedesktop.org  |




--- Comment #2 from Sjoerd Simons <sjoerd at luon.net>  2009-11-30 05:04:47 PST ---
9034d0c67a7f2928490f2640f41e8db9c6d13bac:
  Do you cope with the semantic change that when the factory is disposed with
GibberResolver the callback isn't called, while with the GResolver the
operation is cancelled but the callback is still called ?

e757c00715fcdc728913d985676515b3784cae20:
+#ifdef G_LOG_DOMAIN
+#undef G_LOG_DOMAIN
+#endif
+#define G_LOG_DOMAIN "test-resolver"

uh what now? should probably use the standard gabble/wocky debug framework..

+enum
+{
+  PROP_REAL_RESOLVER = 1,
+};

It should never chain up to a real resolver, as that would mean the test could
depend on the network.

+typedef struct _fake_host { char *key; char *addr; } fake_host;
+typedef struct _fake_serv { char *key; GSrvTarget *srv; } fake_serv;

Not telepathy style..

+static gchar *
+_service_rrname (const char *service,
+                 const char *protocol,
+                 const char *domain)
+{
+  gchar *rrname, *ascii_domain = NULL;
+
+  if (g_hostname_is_non_ascii (domain))
+    domain = ascii_domain = g_hostname_to_ascii (domain);
+
+  rrname = g_strdup_printf ("_%s._%s.%s", service, protocol, domain);
+
+  g_free (ascii_domain);
+  return rrname;
+}

not bothering with g_h_is_non_ascii would probably make the code a bit nicer

++static GList *
+find_fake_services (TestResolver *tr, const char *name)
use gchar instead of char
+{
+  GList *fake = NULL;
+  GList *rval = NULL;
+
+  for (fake = tr->fake_SRV; fake; fake = fake->next)
  Use g_list_next and fake != NULL;

+    {
+      fake_serv *entry = fake->data;
+      if (entry && !g_strcmp0 (entry->key, name))
 Can't we assert entry != NULL and if not use entry != NULL, not entry

+          rval = g_list_append (rval, g_srv_target_copy (entry->srv));
+    }
+  return rval;
+}
+
+static GList *
+find_fake_hosts (TestResolver *tr, const char *name)
+{
+  GList *fake = NULL;
+  GList *rval = NULL;
+
+  for (fake = tr->fake_A; fake; fake = fake->next)
+    {
+      fake_host *entry = fake->data;
+      if (entry && !g_strcmp0 (entry->key, name))
+        rval =
+          g_list_append (rval, g_inet_address_new_from_string (entry->addr));
+    }
+  return rval;
+}
Same comment as in find_fake_services

+static void
+lookup_service_async (GResolver *resolver,
+    const char *rr,
+    GCancellable *cancellable,
+    GAsyncReadyCallback  cb,
+    gpointer data)
+{
+  GError *error = NULL;
+  TestResolver *tr = TEST_RESOLVER (resolver);
+  GList *addr = find_fake_services (tr, rr);
+  GObject *source = G_OBJECT (resolver);
+  GSimpleAsyncResult *res = NULL;
+#ifdef DEBUG_FAKEDNS
+  GList *x;
+#endif

debugging shouldn't be a special compile time function, should probably use the
same debugging infrastructure as in wocky/gabble if we want any debugging in it
(probably not needed)

+  if (addr == NULL)
+    {
+      GResolver *real = G_RESOLVER (tr->real_resolver);
+      addr = G_RESOLVER_GET_CLASS (real)->
+        lookup_service (real, rr, cancellable, &error);
Don't chain up to a real resolver. we've fixed this in wocky, but not done some
other cleanup yet, a resync might be good

+    }
+#ifdef DEBUG_FAKEDNS
+  else
+    for (x = addr; x; x = x->next)
+      g_debug ("FAKE SRV: addr: %s; port: %d; prio: %d; weight: %d;\n",
+          g_srv_target_get_hostname ((GSrvTarget *) x->data),
+          g_srv_target_get_port ((GSrvTarget *) x->data),
+          g_srv_target_get_priority ((GSrvTarget *) x->data),
+          g_srv_target_get_weight ((GSrvTarget *) x->data));
+#endif
+
+  if (addr != NULL)
+      res = g_simple_async_result_new (source, cb, data,
lookup_service_async);
+  else
+      res = g_simple_async_result_new_from_error (source, cb, data, error);
+
+  g_simple_async_result_set_op_res_gpointer (res, addr, NULL);

The code would be a lot easier to understnad if it did:
  res = g_s_a_r_new ()
  if (addr != NULL)
    g_s_a_r_set_op_res_g ()
  else
    g_s_a_r_set_error ()

+  g_simple_async_result_complete (res);
should be complete_in_idle 

+static GList *
+lookup_service_finish (GResolver *resolver,
+                       GAsyncResult *result,
+                       GError **error)
+{
+  GList *res = NULL;
+  GSimpleAsyncResult *simple = G_SIMPLE_ASYNC_RESULT (result);
+  g_simple_async_result_propagate_error (simple, error);
+  res = g_simple_async_result_get_op_res_gpointer (simple);

Would be better to use the normal boilerplate for this.. Also don't assme
get_op_res_gpointer is NULL when an error is set..

+static void
+lookup_by_name_async (GResolver *resolver,
+                      const gchar *hostname,
+                      GCancellable *cancellable,
+                      GAsyncReadyCallback  cb,
+                      gpointer data)
+{
+  GError *error = NULL;
+  TestResolver *tr = TEST_RESOLVER (resolver);
+  GList *addr = find_fake_hosts (tr, hostname);
+  GObject *source = G_OBJECT (resolver);
+  GSimpleAsyncResult *res = NULL;
+#ifdef DEBUG_FAKEDNS
+  GList *x;
+  char a[32];
+#endif
+
+  if (addr == NULL)
+    {
+      GResolver *real = G_RESOLVER (tr->real_resolver);
+      addr = g_resolver_lookup_by_name (real, hostname, cancellable, &error);
         don't chain up..
+    }
+#ifdef DEBUG_FAKEDNS
+  else
+    for (x = addr; x; x = x->next)
+      g_debug ("FAKE HOST: addr: %s;\n",
+          inet_ntop (AF_INET,
+              g_inet_address_to_bytes (x->data), a, sizeof (a)));
+#endif
more debugging crap...

+  if (addr != NULL)
+      res = g_simple_async_result_new (source, cb, data,
lookup_service_async);
+  else
+      res = g_simple_async_result_new_from_error (source, cb, data, error);
+
+  g_simple_async_result_set_op_res_gpointer (res, addr, NULL);

Same comments about laying out the code as above

+static GList *
+lookup_by_name_finish (GResolver *resolver,
+    GAsyncResult *result,
+    GError **error)
+{
+  GList *res = NULL;
+  GSimpleAsyncResult *simple = G_SIMPLE_ASYNC_RESULT (result);
+  g_simple_async_result_propagate_error (simple, error);
+  res = g_simple_async_result_get_op_res_gpointer (simple);
+  return res;

same comment as above, use the normal boilerplate, don't mak assumption about
op_res_pointer

+static void
+test_resolver_class_init (TestResolverClass *klass)
+{
+  GResolverClass *resolver_class = G_RESOLVER_CLASS (klass);
+  GObjectClass *object_class = G_OBJECT_CLASS (klass);
+  GParamSpec *spec;
+
+  object_class->set_property = test_resolver_set_property;
+  object_class->get_property = test_resolver_get_property;
+
+  spec = g_param_spec_object ("real-resolver", "real-resolver",
+      "The real resolver to use when we don't have a kludge entry",
+      G_TYPE_RESOLVER,
+      G_PARAM_READWRITE | G_PARAM_STATIC_STRINGS | G_PARAM_CONSTRUCT);

die real-resolver, die :) also means we can get rid of the getter and setter

+void
+test_resolver_reset (TestResolver *tr)
+{
+  GList *fake = NULL;
+
+  for (fake = tr->fake_A; fake; fake = fake->next)
use fake != NULL, and use g_list_next.
+    {
+      fake_host *entry = fake->data;
+      g_free (entry->key);
+      g_free (entry->addr);
+      g_free (entry);
+    }
+  g_list_free (tr->fake_A);
+  tr->fake_A = NULL;
+
+  for (fake = tr->fake_SRV; fake; fake = fake->next)
ditto
+    {
+      fake_serv *entry = fake->data;
+      g_free (entry->key);
+      g_srv_target_free (entry->srv);
+      g_free (entry);
+    }
+  g_list_free (tr->fake_SRV);
+  tr->fake_SRV = NULL;
+}


-- 
Configure bugmail: http://bugs.freedesktop.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.



More information about the telepathy-bugs mailing list