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 795596 - rtspconnection: GError set over the top of a previous GError or uninitialized memory.
rtspconnection: GError set over the top of a previous GError or uninitialized...
Status: RESOLVED OBSOLETE
Product: GStreamer
Classification: Platform
Component: gst-plugins-base
git master
Other Linux
: Normal normal
: git master
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2018-04-27 07:15 UTC by Nils Öhnell
Modified: 2018-11-03 12:06 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Patch to address the problem (4.06 KB, patch)
2018-04-27 10:03 UTC, Nils Öhnell
none Details | Review
Patch to address the problem (4.04 KB, patch)
2018-04-27 12:01 UTC, Nils Öhnell
none Details | Review
Obsolete patch 371457 (43 bytes, text/plain)
2018-05-04 11:46 UTC, Nils Öhnell
  Details
gstrtspconnection.c: Error handling in write_bytes() and read_bytes() (3.87 KB, patch)
2018-05-16 08:49 UTC, Nils Öhnell
none Details | Review

Description Nils Öhnell 2018-04-27 07:15:39 UTC
file: gstrtspconnection.c
functions: read_bytes(), write_bytes(), fill_bytes()

The functions do not have handling for the situation that sub-functions return both a positive number of bytes and an error at the same time.

The problem is discovered in the next lap of the while-loop when the error has not been cleared from the previous lap, and this error print is issued:
"GError set over the top of a previous GError or uninitialized memory."
Comment 1 Nils Öhnell 2018-04-27 10:03:11 UTC
Created attachment 371454 [details] [review]
Patch to address the problem

The proposed patch goes to error not only when returned number of bytes is non-negative but also when err is set.
This change required some rework of the error handling.
The function write_bytes() is aligned to the read_byte() function for handling the case where returned bytes is zero.
Comment 2 Nils Öhnell 2018-04-27 12:01:14 UTC
Created attachment 371457 [details] [review]
Patch to address the problem

Same comment as for obsoleted attachment 371454 [details] [review].
The only difference is an updated print format specifier.
Comment 3 Nils Öhnell 2018-05-04 11:46:06 UTC
Created attachment 371668 [details]
Obsolete patch 371457

Problem with rtsph detected.
The current patch is obsoleted.
A new patch will be provided when verified.
Comment 4 Nils Öhnell 2018-05-16 08:49:42 UTC
Created attachment 372114 [details] [review]
gstrtspconnection.c: Error handling in write_bytes() and read_bytes()

The proposed patch goes to error handling not only based on the return value from functions but also when err is set.

The change required some rework of the error handling.
Comment 5 Nils Öhnell 2018-08-15 06:08:55 UTC
Has anyone had a chance to look at this yet?
Comment 6 Tim-Philipp Müller 2018-08-15 18:23:33 UTC
Right, so I see that the warning/critical needs fixing, and I understand how that happens.

What is not entirely clear to me yet if this is the right change.

What error is being returned in this case where it returns a non-negative number as well?

Is this intentional and expected or perhaps a bug in the underlying code in GLib?
Comment 7 Nils Öhnell 2018-08-22 09:54:50 UTC
> What error is being returned in this case where it returns a non-negative number as well?
The returned error is depending on the G_IO_ERROR with GST_RTSP_ESYS as default, as before.

> Is this intentional and expected or perhaps a bug in the underlying code in GLib?
Well, the GLib documentation states "the number of bytes ..., or -1 on error" for both the underlying GLib functions (g_pollable_output_stream_write_nonblocking and g_pollable_input_stream_read_nonblocking), so at least one could say that the GLib implementation does not match the GLib documentation.
Comment 8 Sebastian Dröge (slomo) 2018-08-27 06:09:56 UTC
(In reply to Nils Öhnell from comment #7)
> > What error is being returned in this case where it returns a non-negative number as well?
> The returned error is depending on the G_IO_ERROR with GST_RTSP_ESYS as
> default, as before.

I think the question was what the actual GIO error is that happens here

> > Is this intentional and expected or perhaps a bug in the underlying code in GLib?
> Well, the GLib documentation states "the number of bytes ..., or -1 on
> error" for both the underlying GLib functions
> (g_pollable_output_stream_write_nonblocking and
> g_pollable_input_stream_read_nonblocking), so at least one could say that
> the GLib implementation does not match the GLib documentation.

Can you file a bug about that again GLib? This is definitely not correct, both for the given documentation and the behaviour that code relies on everywhere with regards to these functions.

Does this happen with direct TCP connections, or via TLS, or ...?
Comment 9 GStreamer system administrator 2018-11-03 12:06:27 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/gst-plugins-base/issues/445.