On Thu, 2005-05-05 at 10:15 +0200, Tim Janik wrote:
> 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)); \
> > +} G_STMT_END
>
> 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)); \
> > +} G_STMT_END
>
> why is this not simply (GData*) (~flags & (gsize) *(datalist)) ?
OK, yes, these just got crudded up, the above should work. Well, except
that going back to our earlier discussion
~flags & (gsize) *(datalist)
*doesn't* work, because gsize might be wider than an int. It needs to be
~(gsize)(flags) & (gsize) *(datalist)
Right? Those unnecessary casts aren't *always* unnecessary :-)
> > +#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); \
> > +} G_STMT_END
>
> 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.
Fixed up, gobject docs moved out-of-line, committed.
Regards,
Owen
Attachment:
signature.asc
Description: This is a digitally signed message part