[Webkit-unassigned] [Bug 30658] Uninitialized memory access in svg SynchronizableProperty hashing

[bugzilla-daemon at webkit.org]

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


Darin Adler <darin at apple.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #41630|review?                     |review-
               Flag|                            |




--- Comment #4 from Darin Adler <darin at apple.com>  2009-10-25 15:54:07 PDT ---
(From update of attachment 41630)
If the whole point here is to record a set of properties and a flag for which
ones need synchronization, then I suggest using a
HashMap<SVGAnimatedPropertyBase*, bool> rather than a HashSet of a new type.
Then we could get rid of SynchronizableProperty, SynchronizablePropertyHash,
and SynchronizablePropertyHashTraits.

Another option would be to use two HashSet<SVGAnimatedPropertyBase*> -- one for
all properties, and another that contains only properties that require
synchronization. That could still be done without a custom hash.

>      bool operator==(const SynchronizableProperty& other) const
>      {
> -        return base == other.base && shouldSynchronize == other.shouldSynchronize;
> +        return base == other.base;
>      }

If we don't take my suggestion above, then I think it's poor style to have an
"==" operation that will cause two object to compare as equal if some of their
state is not identical. I suggest changing SynchronizablePropertyHash::equal
instead. I also suggest removing this overload of operator== since it is
unlikely there is code that relies on it.

>      static unsigned hash(const SynchronizableProperty& key)
>      {
> -        return StringImpl::computeHash(reinterpret_cast<const UChar*>(&key), sizeof(SynchronizableProperty) / sizeof(UChar));
> +        return StringImpl::computeHash(reinterpret_cast<const UChar*>(&key.base), sizeof(key.base) / sizeof(UChar));
>      }

If we don't take my suggestion above, then since we are hashing just a pointer,
it's far better to use PtrHash<SVGAnimatedPropertyBase*>::hash instead of
StringImpl::computeHash.

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