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 308647 - [filesink] not handling errors properly?
[filesink] not handling errors properly?
Status: RESOLVED INVALID
Product: GStreamer
Classification: Platform
Component: gstreamer (core)
git master
Other Linux
: Normal normal
: 0.8.11
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2005-06-22 13:37 UTC by Michal Benes
Modified: 2005-06-27 10:29 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
change size_t to ssize_t (458 bytes, patch)
2005-06-22 13:39 UTC, Michal Benes
none Details | Review
proposed patch (1.53 KB, patch)
2005-06-26 18:59 UTC, Tim-Philipp Müller
none Details | Review

Description Michal Benes 2005-06-22 13:37:33 UTC
When fwrite fails (disk full or other problem) the -1 error code is store to
size_t which is unsigned and filesink does not recognize there was an error.
Comment 1 Michal Benes 2005-06-22 13:39:41 UTC
Created attachment 48156 [details] [review]
change size_t to ssize_t

This patch changes type of wrote variable to ssize_t so that condition wrote <=
0 makes better sense.
Comment 2 Luca Ognibene 2005-06-25 11:40:05 UTC
uhm, from man fwrite (Ubuntu hoary):

size_t fwrite(const void *ptr, size_t size, size_t nmemb, FILE *stream);

RETURN VALUE
       fread and fwrite return the number of items successfully read or written
(i.e., not the number of characters).  If
       an error occurs, or the end-of-file is reached, the return value is a
short item count (or zero).

       fread does not distinguish between end-of-file and error, and callers
must use feof(3) and ferror(3) to  determine
       which occurred.

I think the check should be changed to:
if (ferror (filesink->file)) {
   GST_ELEMENT_ERROR (...)
Comment 3 Tim-Philipp Müller 2005-06-26 18:59:56 UTC
Created attachment 48370 [details] [review]
proposed patch

The code in question is unnecessarily complicated anyway. With fwrite() you
don't really need that whole loop business like you do with write(). fwrite()
writes N elements of size X and returns the number of elements written, so we
just write 1 element of size GST_BUFFER_SIZE and either get a return value of 1
or 0. The operation will either succeed or fail, and we don't need to handle
short writes and all that.

Proposed patch attached.

Cheers
 -Tim
Comment 4 Tim-Philipp Müller 2005-06-26 19:11:25 UTC
Forgot to add:

Do you actually have issues with filesink not recognising errors? It errors out
fine for me if there's no space left on the device, and I don't see anything in
the current code that would prevent it from recognising errors either (and if
only in the next while loop iteration).

The only thing I did observe is that there seems to be some kind of delay before
the no-space-left-on-device error is emitted (ie. it writes more than there's
actually space), but I figured this might be because of buffering lower in the
stack or because of certain filesystem characteristics (ext3 here) *shrug*

If you do have issues, maybe strace can shed some light on what's going on.

Cheers
 -Tim
Comment 5 Michal Benes 2005-06-27 07:40:07 UTC
Ooops, I am very sorry, I was reading man page of write (2), which I was using
in my derivative of filesink. So the answer is, I had not problems with filesink
itself, but with my derivative of it.