diff mbox

[3/6] QDict: Introduce qdict_copy()

Message ID 1328902266-25308-4-git-send-email-lcapitulino@redhat.com
State New
Headers show

Commit Message

Luiz Capitulino Feb. 10, 2012, 7:31 p.m. UTC 
Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
 check-qdict.c |   29 +++++++++++++++++++++++++++++
 qdict.c       |   18 ++++++++++++++++++
 qdict.h       |    1 +
 3 files changed, 48 insertions(+), 0 deletions(-)

Comments

Juan Quintela Feb. 15, 2012, 1:10 p.m. UTC | #1 
Luiz Capitulino <lcapitulino@redhat.com> wrote:
> Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>


>  /**
> + * qdict_copy(): Build a new dictionary from an existing one.
> + */
> +QDict *qdict_copy(const QDict *from)
> +{
> +    const QDictEntry *ent;
> +    QDict *new;
> +
> +    new = qdict_new();
> +
> +    for (ent = qdict_first(from); ent; ent = qdict_next(from, ent)) {
> +        qdict_put_obj(new, qdict_entry_key(ent), qdict_entry_value(ent));
> +        qobject_incref(qdict_entry_value(ent));
> +    }


Without having any clue about how qobject refcounting works,  it looks
suspicious that we first insert an object in a dict, and increase the
ref counter after.  Shouldn't we do it the other way around?

Later, Juan.
Luiz Capitulino Feb. 15, 2012, 5:01 p.m. UTC | #2 
On Wed, 15 Feb 2012 14:10:32 +0100
Juan Quintela <quintela@redhat.com> wrote:

> Luiz Capitulino <lcapitulino@redhat.com> wrote:
> > Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
> 
> 
> >  /**
> > + * qdict_copy(): Build a new dictionary from an existing one.
> > + */
> > +QDict *qdict_copy(const QDict *from)
> > +{
> > +    const QDictEntry *ent;
> > +    QDict *new;
> > +
> > +    new = qdict_new();
> > +
> > +    for (ent = qdict_first(from); ent; ent = qdict_next(from, ent)) {
> > +        qdict_put_obj(new, qdict_entry_key(ent), qdict_entry_value(ent));
> > +        qobject_incref(qdict_entry_value(ent));
> > +    }
> 
> 
> Without having any clue about how qobject refcounting works,  it looks
> suspicious that we first insert an object in a dict, and increase the
> ref counter after.  Shouldn't we do it the other way around?

I think this only matters for threaded applications. QDicts are not thread
safe, so this is not an issue but I'll drop this patch anyway (in favor of
increfing the error object).
diff mbox

Patch

diff --git a/check-qdict.c b/check-qdict.c
index fc0d276..12c73ab 100644
--- a/check-qdict.c
+++ b/check-qdict.c
@@ -227,6 +227,34 @@  static void qdict_iterapi_test(void)
     QDECREF(tests_dict);
 }
 
+static void qdict_copy_test(void)
+{
+    QDict *tests_dict = qdict_new();
+    const QDictEntry *ent;
+    QDict *new;
+    int i;
+
+    for (i = 0; i < 1024; i++) {
+        char key[32];
+
+        snprintf(key, sizeof(key), "key%d", i);
+        qdict_put(tests_dict, key, qint_from_int(i));
+    }
+
+    new = qdict_copy(tests_dict);
+
+    g_assert(qdict_size(new) == qdict_size(tests_dict));
+    for (ent = qdict_first(tests_dict); ent; ent = qdict_next(tests_dict, ent)){
+        const char *key = qdict_entry_key(ent);
+
+        g_assert(qdict_haskey(new, key) == 1);
+        g_assert(qdict_get_int(new, key) == qdict_get_int(tests_dict, key));
+    }
+
+    QDECREF(new);
+    QDECREF(tests_dict);
+}
+
 /*
  * Errors test-cases
  */
@@ -365,6 +393,7 @@  int main(int argc, char **argv)
     g_test_add_func("/public/del", qdict_del_test);
     g_test_add_func("/public/to_qdict", qobject_to_qdict_test);
     g_test_add_func("/public/iterapi", qdict_iterapi_test);
+    g_test_add_func("/public/copy", qdict_copy_test);
 
     g_test_add_func("/errors/put_exists", qdict_put_exists_test);
     g_test_add_func("/errors/get_not_exists", qdict_get_not_exists_test);
diff --git a/qdict.c b/qdict.c
index 4bf308b..dc9141f 100644
--- a/qdict.c
+++ b/qdict.c
@@ -401,6 +401,24 @@  const QDictEntry *qdict_next(const QDict *qdict, const QDictEntry *entry)
 }
 
 /**
+ * qdict_copy(): Build a new dictionary from an existing one.
+ */
+QDict *qdict_copy(const QDict *from)
+{
+    const QDictEntry *ent;
+    QDict *new;
+
+    new = qdict_new();
+
+    for (ent = qdict_first(from); ent; ent = qdict_next(from, ent)) {
+        qdict_put_obj(new, qdict_entry_key(ent), qdict_entry_value(ent));
+        qobject_incref(qdict_entry_value(ent));
+    }
+
+    return new;
+}
+
+/**
  * qentry_destroy(): Free all the memory allocated by a QDictEntry
  */
 static void qentry_destroy(QDictEntry *e)
diff --git a/qdict.h b/qdict.h
index 929d8d2..9976a18 100644
--- a/qdict.h
+++ b/qdict.h
@@ -36,6 +36,7 @@  typedef struct QDict {
 QDict *qdict_new(void);
 const char *qdict_entry_key(const QDictEntry *entry);
 QObject *qdict_entry_value(const QDictEntry *entry);
+QDict *qdict_copy(const QDict *from);
 size_t qdict_size(const QDict *qdict);
 void qdict_put_obj(QDict *qdict, const char *key, QObject *value);
 void qdict_del(QDict *qdict, const char *key);