Re: Introducing "toggle references"

On Sat, 30 Apr 2005, Owen Taylor wrote:

On Sat, 2005-04-30 at 13:56 +0200, Tim Janik wrote:
On Thu, 28 Apr 2005, Owen Taylor wrote:

On Wed, 2005-04-27 at 20:07 -0400, Owen Taylor wrote:

I'll do a new version tomorrow.

Here's a new version. I think it addresses all the points that
were discussed.

Two other changes in the new version:

- in gdatasetprivate.h gsize instead of gulong is used for
  pointer => int casts. Really we'd want uinptr_t, but
  gsize works on all platforms I know. (gulong apparently
  won't work on win64)

- The docs for g_object_add_weak_ref() are extended to
  describe the use case in some detail.

the patch only contained the gobject/ part of your changes.
i'm fine with those but i'd also like to see the final dataset.*
changeset ;)

Let's try that again ...

ok, i assume you didn't change the GObject part since the last patch.
so with the dataset partch i only have minor issues:

+#define G_DATALIST_GET_FLAGS(datalist)                                                 \
+  ((gsize)*(datalist) & G_DATALIST_FLAGS_MASK)
+#define G_DATALIST_SET_FLAGS(datalist, flags) G_STMT_START {                           \
+  *datalist = (GData *)(G_DATALIST_GET_FLAGS(datalist) |                               \
+                       (flags) |                                                       \
+                       ((gsize)*(datalist) & ~(gsize)G_DATALIST_FLAGS_MASK));          \

why is this not simply (GData*) (flags | (gsize) *(datalist)) ?

+#define G_DATALIST_UNSET_FLAGS(datalist, flags) G_STMT_START {                         \
+  *datalist = (GData *)((G_DATALIST_GET_FLAGS(datalist) & ~(flags)) |                  \
+                       (flags) |                                                       \
+                       ((gsize)*(datalist) & ~(gsize)G_DATALIST_FLAGS_MASK));          \

why is this not simply (GData*) (~flags & (gsize) *(datalist)) ?

+#define G_DATALIST_GET_POINTER(datalist)                                               \
+  ((GData *)((gsize)*(datalist) & ~(gsize)G_DATALIST_FLAGS_MASK))
+#define G_DATALIST_SET_POINTER(datalist,pointer) G_STMT_START {                                \
+  *(datalist) = (GData *)(G_DATALIST_GET_FLAGS (datalist) |                            \
+                         (gsize)pointer);                                              \

it looks to me like SET() and UNSET() became a little over-engineered with
the recent changes and could be simplified. other than that, i'm fine with
the patch being applied.



[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]