[MPlayer-dev-eng] [RFC] always memset structs to 0 in libass

Reimar Döffinger Reimar.Doeffinger at gmx.de
Sat Mar 28 20:16:55 CET 2009


On Sat, Mar 28, 2009 at 10:00:42PM +0300, Evgeniy Stepanov wrote:
> On Saturday 28 March 2009 21:36:54 Reimar Döffinger wrote:
> > Hello,
> > I originally stumbled over this because of valgrind warnings.
> > The real issue is that you can't just use the default
> > hashmap_hash/hashmap_key_compare with structs, but I leave that to the
> > libass maintainer(s).
> > Independent of that, I consider it a good idea to initialize all
> > structs to 0 anyway, simply because
> > 1) it makes it easier to extend those structs without running into weird
> > random bugs
> > 2) for what I can tell there are only exactly _three_ places where this
> > is currently not done, so it makes things more consistent as well.
> >
> > Attached is the proposed patch for this.
> 
> Sure, I also find it a good idea. Btw, your patch contains unrelated changes ;) 
> They are fine, too.

Here is another "unrelated" change (in case you are not aware, structs
may have any kind of padding, and the compiler is basically free to write
any value whenever it likes into that padding):
Index: libass/ass_cache.c
===================================================================
--- libass/ass_cache.c  (revision 29081)
+++ libass/ass_cache.c  (working copy)
@@ -262,6 +262,7 @@
 
 void ass_bitmap_cache_init(void)
 {
+       // FIXME: using generic hash and compare functions on a struct is incorrect due to padding
        bitmap_cache = hashmap_init(sizeof(bitmap_hash_key_t),
                                   sizeof(bitmap_hash_val_t),
                                   0xFFFF + 13,
@@ -310,6 +311,7 @@
 
 void ass_glyph_cache_init(void)
 {
+       // FIXME: using generic hash and compare functions on a struct is incorrect due to padding
        glyph_cache = hashmap_init(sizeof(glyph_hash_key_t),
                                   sizeof(glyph_hash_val_t),
                                   0xFFFF + 13,
@@ -359,6 +361,7 @@
 
 void ass_composite_cache_init(void)
 {
+       // FIXME: using generic hash and compare functions on a struct is incorrect due to padding
        composite_cache = hashmap_init(sizeof(composite_hash_key_t),
                                   sizeof(composite_hash_val_t),
                                   0xFFFF + 13,



More information about the MPlayer-dev-eng mailing list