This is the mail archive of the ecos-patches@sourceware.org mailing list for the eCos project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [ECOS] fseek on JFFS2


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


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]