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 345503 - patch to better handle stream-filter sources in a stream-cat
patch to better handle stream-filter sources in a stream-cat
Status: RESOLVED FIXED
Product: gmime
Classification: Other
Component: general
unspecified
Other Linux
: Normal normal
: ---
Assigned To: Jeffrey Stedfast
Jeffrey Stedfast
Depends on:
Blocks:
 
 
Reported: 2006-06-21 01:39 UTC by Charles Kerr
Modified: 2006-06-23 18:12 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
cat.patch (765 bytes, patch)
2006-06-21 22:18 UTC, Jeffrey Stedfast
none Details | Review

Description Charles Kerr 2006-06-21 01:39:26 UTC
Before a cat stream starts reading, it tries to seek to the right position
in the stream by calling seek for the right nested source.  Seeking isn't
possible in a stream_filter for obvious reasons, so calling read() on the
cat stream fails.

Arbitrary seeking _is_ impractical for a filter stream, but in the special
case of a 'prep' seek for reading the filter from beginning to end it
devolves into a call to stream_reset(), so reading a cat stream from
beginning to end, even if it has filter stream children, is doable:

--- gmime-stream-filter.c.bak   2006-06-20 20:20:44.000000000 -0500
+++ gmime-stream-filter.c       2006-06-20 20:32:24.000000000 -0500
@@ -326,6 +326,12 @@
 static off_t
 stream_seek (GMimeStream *stream, off_t offset, GMimeSeekWhence whence)
 {
+       if (whence==SEEK_SET && offset==stream->bound_start)
+       {
+               stream_reset (stream);
+               return 0;
+       }
+
        return -1;
 }
Comment 1 Jeffrey Stedfast 2006-06-21 22:18:32 UTC
Created attachment 67820 [details] [review]
cat.patch

would this work? I'd rather if seek doesn't work for all seeks on a stream class, then it not work at all on said class
Comment 2 Charles Kerr 2006-06-21 22:25:46 UTC
That wouldn't work as-is, but I agree it's a better approach.

We need to test reset if real==bound_start, not if real==0. 

if (real == current->stream->bound_start) ret = reset...
else if (real > 0) ret = seek ...
else ret = -1;
Comment 3 Jeffrey Stedfast 2006-06-22 14:41:35 UTC
doh, duh... yea - silly me :)

ok, with more consideration I realised that simply handling the real==bound_start case won't necessarily be enough - it needs to handle where real==position too.

so here's what I came up with:

if (current->stream->position != real) {
	if (real > current->stream->bound_start) {
		/* no choice but to seek */
		ret = g_mime_stream_seek (current->stream, real, GMIME_STREAM_SEEK_SET);
	} else if (real == current->stream->bound_start) {
		/* special-case: reset() allows all streams (seekable or not) to reset
		 * their state and positions to bound_start */
		ret = g_mime_stream_reset (current->stream);
	} else {
		ret = -1;
	}
} else {
	/* no seek required */
	ret = 0;
}
Comment 4 Charles Kerr 2006-06-23 04:37:18 UTC
looks very good to me.  Only change I'd do is remove a
nesting by negating the first test.

  if (position == real) {
    ret = 0;
  else if (real > bound_start)
    ret = seek;
  else if (real == bound_start)
    ret = reset;
  else
    ret = -1;
  
Comment 5 Jeffrey Stedfast 2006-06-23 18:12:45 UTC
committed