[Webkit-unassigned] [Bug 127246] [GLIB] GVariant floating references are not correctly handled by GRefPtr

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=127246


Adrian Perez <aperez at igalia.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |aperez at igalia.com




--- Comment #2 from Adrian Perez <aperez at igalia.com>  2014-01-19 08:05:31 PST ---
(In reply to comment #0)
> GRefPtr should always use g_variant_ref_sink to deal with GVariant floating references. In case of full references, g_variant_ref_sink calls g_variant_ref, so it's safe to use it always.

Being this the case, if I am understanding GRefPtr correctly then we should
never use adoptGRef() for GVariants, because it does *not* sink the floating
reference or add a reference. If that's the case it would be good to add
a specialization of the adoptGRef() template to make code that uses it
work properly with GVariants. I see two possible ways:

 a. Specialize adoptGRef<GVariant>() to *not* pass GRefPtrAdopt as second
    argument. This would make adoptGRef() sink the floating reference:

      template<> GRefPtr<GVariant> adoptGRef(GVariant* p) {
          return GRefPtr<GVariant>(p);
      }

 b. Make the compiler stop with error when using adoptGRef<GVariant>().
    For example using a “static_assert” depending on the type used for
    GRefPtr. This would prevent using adoptGRef() for GVariants. For
    example the following would cause compiler errors when instantiating
    adoptGRef() for GVariants:

      // In general, adoptGRef() can be used for any type.
      template <typename T> class GRefPtr {
          // ...
          static const bool canUseAdoptGRef = true;
      };

      // But not for GVariant, so specialize the value of the flag.
      template <> class GRefPtr<GVariant> {
          static const bool canUseAdoptGRef = false;
      };

      template <typename T> GRefPtr<T> adoptGRef(T* p) {
          static_assert(GRefPtr<T>::canUseAdoptGRef,
                        "Can't use adoptGRef() for this type");
          return GRefPtr<T>(p, GRefPtrAdopt);
      }

Probably option (a) is the less intrusive, and it would avoid having to go
through the existing code chasing the existing uses of adoptGRef(). WDYT?

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