[ECOS] fseek on JFFS2

[Jonathan Larmour]

Ivan Djelic wrote:
> Hello all,
> 
> I believe the problems you are discussing may be related to several bugs I
> found in Cyg_StdioStream::set_position():
> 
> 1) Type 'fpos_t' of parameter 'pos' is defined as cyg_ucount32, whereas
> 'pos' is generally assumed to be signed; which triggers a few annoying bugs.
> For instance, if you do something like:
> 
>  fgetc(fp);
>  fseek( fp, -1, SEEK_CUR );
> 
>  then set_position() gets called with pos = 0xffffffff (unsigned).
> 
>  'fpos_t' could probably be defined as cyg_count32, at least that's
>  how I fixed the problem.

Ok.

> 2) When CYGPKG_LIBC_STDIO_FILEIO is used, there is a piece of code in
> set_position() which (as was said in some earlier post):
> "calculates the new absolute position and calls cyg_stdio_lseek()".
> Unfortunately it does not work with the following example:
> 
>   fgetc(fp);
>   fseek( fp, -1, SEEK_CUR );
> 
> Assuming the stream uses buffers of 256 bytes, and using the 'sfp'/'ufp'
> notation introduced in earlier posts, we have:
> 
> before fgetc() (assume we just opened the file):
>   sfp = 0
>   ufp = 0
> 
> after fgetc():
>   sfp = 1
>   ufp = 0x100 (because we read a full buffer)
> 
> At this point, set_position() is called with:
>   whence = SEEK_CUR
>   pos = -1 (or 0xffffffff is fpos_t is cyg_ucount32, it does not really change
>             anything here)
> 
> The following variables are computed:
>   abspos = position + pos;
>   posdiff = abspos - position; ( = pos )
> 
> -> posdiff is equal to -1
> 
> then we have:
> 
>   position += bytesavail;
> 
> so that 'position' is now equal to ufp (0x100).
> 
> Then,
> 
>   cyg_stdio_lseek( my_device, &newpos, whence );
> 
> with newpos == -1, whence == SEEK_CUR. Remember that ufp = 0x100.
> 
> and finally 'position' is updated:
> 
>   position = newpos; (= 0xff)
> 
> Finally, we have:
> 
> sfp = 0xff
> ufp = 0xff
> 
> But this is *not* what we want ! We want to have sfp = 0, so that for instance
> ftell(fp) returns 0.
> The problem is that we forced sfp to be equal to ufp before seeking, whereas
> it should have been the other way around.
> Here is my fix:
> 
> 
> @@ -441,7 +441,9 @@ Cyg_StdioStream::set_position( fpos_t po
>          } // endif (bytesavail > posdiff)
>  
>          if (whence == SEEK_CUR) {
> -            position += bytesavail;
> +          pos += position;
> +          whence = SEEK_SET;
>          }
>      } //endif (whence != SEEK_END)
>  
> 
> That way, we convert 'pos' to an offset from the beginning of the stream
> (SEEK_SET), and we do not depend on the value of ufp when seeking... 

I agree with your analysis, but I think I would prefer a fix like:

Index: include/stream.inl
===================================================================
RCS file: 
/cvs/ecos/ecos/packages/language/c/libc/stdio/current/include/stream.inl,v
retrieving revision 1.7
diff -u -5 -p -r1.7 stream.inl
--- include/stream.inl  29 Mar 2004 11:24:38 -0000      1.7
+++ include/stream.inl  27 Sep 2006 15:14:52 -0000
@@ -440,10 +440,11 @@ Cyg_StdioStream::set_position( fpos_t po
              return ENOERR;
          } // endif (bytesavail > posdiff)

          if (whence == SEEK_CUR) {
              position += bytesavail;
+            pos -= bytesavail;
          }
      } //endif (whence != SEEK_END)

      Cyg_ErrNo err;

What do you think?

Jifl
-- 
eCosCentric    http://www.eCosCentric.com/    The eCos and RedBoot experts
------["The best things in life aren't things."]------      Opinions==mine

-- 
Before posting, please read the FAQ: http://ecos.sourceware.org/fom/ecos
and search the list archive: http://ecos.sourceware.org/ml/ecos-discuss