After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 755822 - test_ABI: failed ABI check
test_ABI: failed ABI check
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gstreamer (core)
git master
Other Linux
: Normal normal
: 1.7.1
Assigned To: Stefan Sauer (gstreamer, gtkdoc dev)
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2015-09-30 01:32 UTC by Vineeth
Modified: 2015-10-01 00:31 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
change the size of GstControlPoint to 40 (6.24 KB, patch)
2015-09-30 01:36 UTC, Vineeth
rejected Details | Review
controlpoint: change the padding to be of arch-independent size (7.16 KB, patch)
2015-09-30 15:34 UTC, Stefan Sauer (gstreamer, gtkdoc dev)
committed Details | Review

Description Vineeth 2015-09-30 01:32:00 UTC
When running make check got gstreamer, get the below failure...

gstcheck.c:778:F:size check:test_ABI:0: failed ABI check
Running suite(s): LibsABI
sizeof(GstControlPoint) is 40, expected 48


This happens because,
    struct {
      gdouble c1s, c2s, c3s;
    } cubic_mono;
is added in cache union
The union already had a struct cubic with two double values.
So on adding this structure cubic_mono, the size of the union will increase by only one double value(which is 8).
So the size increase should have been increased from 32-->40. But it was changed to 48

Another reserved variable     gpointer _gst_reserved[GST_PADDING]; is also added in the union. But the size of this is less than that of cubic_mono. So it does not affect the overall size.
Comment 1 Vineeth 2015-09-30 01:36:38 UTC
Created attachment 312381 [details] [review]
change the size of GstControlPoint to 40

Please review..
Comment 2 Luis de Bethencourt 2015-09-30 13:24:55 UTC
I can't reproduce the issue :(
Comment 3 Luis de Bethencourt 2015-09-30 13:28:18 UTC
... but the issue happens if I *apply* your patch.

Size 40 works well here.
Comment 4 Stefan Sauer (gstreamer, gtkdoc dev) 2015-09-30 13:40:03 UTC
Vineeth, on which platform are you testing this? Just asking so that we change this for the right ones.

from ./gst/gstconfig.h:66 we can see that GST_PADDING is defined to 4. Hence a 
gpointer _gst_reserved[GST_PADDING]
is 
32bit platform: 4*4 = 16
64bit platform: 8*4 = 32

we already have 16 bytes outside of the union, the final sizes should be 32 or 48. I don't see how alignment would cause any of them to be 40.

In any case I am not happy with the union there at all, since the 3 doubles (24) are already larger than the padding and hence this is moot.

I've tried to fix this in
d4f81fb4e62d34a4c1dabc65b23ede7ce7694c63
but was assuming that GST_PADDING is perhaps 10 or something reasonable save.
Comment 5 Stefan Sauer (gstreamer, gtkdoc dev) 2015-09-30 13:43:10 UTC
FYI: I am on 64bit x86 and there 48 was what the test told me the struct is.

And now I just realize on 32bit it should be 40 since

32bit platform: 4*4 = 16, but 3 doubles make 24bytes.
Comment 6 Stefan Sauer (gstreamer, gtkdoc dev) 2015-09-30 13:46:49 UTC
As tim mentions there is also GST_PADDING_LARGE, but this would also waste quite some space.

Maybe we just add a double array here that pads the union? That would also be arch-independent? And maybe we have the classic gpointer _reserved[GST_PADDING] at the end.
Comment 7 Stefan Sauer (gstreamer, gtkdoc dev) 2015-09-30 15:34:07 UTC
Created attachment 312436 [details] [review]
controlpoint: change the padding to be of  arch-independent size

This is what would submit to resolve this. If there are no comments against it I'll push it in a few hours to unbreak the abi-test on 32bit platforms.
Comment 8 Vineeth 2015-10-01 00:31:09 UTC
This is a more generic fix and work fine.. thanks :)