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 764288 - structure: Large integer gets detected as double instead of int64
structure: Large integer gets detected as double instead of int64
Status: RESOLVED OBSOLETE
Product: GStreamer
Classification: Platform
Component: gstreamer (core)
git master
Other Linux
: Normal normal
: git master
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2016-03-28 18:50 UTC by Vivia Nikolaidou
Modified: 2018-11-03 12:33 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Test case (983 bytes, text/x-csrc)
2016-03-28 18:50 UTC, Vivia Nikolaidou
  Details
structure: Don't try to parse too large integers as doubles are fractions (3.32 KB, patch)
2016-04-13 08:12 UTC, Sebastian Dröge (slomo)
none Details | Review

Description Vivia Nikolaidou 2016-03-28 18:50:10 UTC
Created attachment 324893 [details]
Test case

Running the attached test case gives this output:

Serialized structure measurement, field1=(int)1000000000;
Structure didn't contain an int64 field1
Serialized structure measurement, field1=(double)10000000000;
Structure didn't contain an int64 field1

Why double? There's no decimal point anywhere, shouldn't it be detected as an int64 instead?

PS: I know it's leaky, it's just a small demo.
PS2: Why is there no GST_STRUCTURE_FORMAT ? Now I'm leaking the strings as well as the structures :(
Comment 1 Sebastian Dröge (slomo) 2016-03-28 18:52:26 UTC
Alternatively it should just fail for things that are bigger than G_MAXINT. Switching from int to int64 (or double) just because of adding another 0 into a string probably caused a few surprises in the past already.

We probably can't change this for 1.x though, might want to warn about it though.
Comment 2 Vincent Penquerc'h 2016-04-12 15:43:55 UTC
Drive by comment, there is a GST_PTR_FORMAT, which works for structures (but within the gst logging API, not g_print).
Comment 3 Sebastian Dröge (slomo) 2016-04-13 06:51:18 UTC
GST_PTR_FORMAT indeed handles GstStructure too, I forgot about that. gst_info_strdup_vprintf() can also handle those btw.
Comment 4 Tim-Philipp Müller 2016-04-13 07:51:31 UTC
We're clearly missing a gst_print() ;)

As for this bug, I would make it bail out with a parsing error if the number is larger than MAXINT. In that case the type just has to be specified manually. I think that's better than magic and doesn't risk people inadvertently reading garbage values later.
Comment 5 Sebastian Dröge (slomo) 2016-04-13 08:12:04 UTC
Created attachment 325844 [details] [review]
structure: Don't try to parse too large integers as doubles are fractions

Having the number of digits decide between different types is going to cause
unexpected surprises.
Comment 6 Sebastian Dröge (slomo) 2016-04-13 08:13:52 UTC
This now still fails... as now the integer is successfully parsed as "string" :) To solve this properly we would need functions to check if something looks like an integer (all digits, optionally starting with a - or +), double (same as integer but with one .) or fraction (same as integer but with one /).

Should we go that route?
Comment 7 GStreamer system administrator 2018-11-03 12:33:54 UTC
-- GitLab Migration Automatic Message --

This bug has been migrated to freedesktop.org's GitLab instance and has been closed from further activity.

You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.freedesktop.org/gstreamer/gstreamer/issues/164.