This is the mail archive of the
ecos-patches@sourceware.org
mailing list for the eCos project.
Re: [ECOS] fseek on JFFS2
- From: Jonathan Larmour <jifl at eCosCentric dot com>
- To: Andrew Lunn <andrew at lunn dot ch>
- Cc: eCos Patches List <ecos-patches at ecos dot sourceware dot org>, Paluch Sebastian <the_sorcerer at op dot pl>, eCos Disuss <ecos-discuss at ecos dot sourceware dot org>
- Date: Tue, 26 Sep 2006 19:45:21 +0100
- Subject: Re: [ECOS] fseek on JFFS2
- References: <op.tf90dvyceuxjss@pls.winproxy> <20060922110323.GC13518@lunn.ch> <op.tf94ugkieuxjss@pls.winproxy> <20060922132509.GA18373@lunn.ch> <20060926075151.GB30214@lunn.ch> <4519409F.6000208@eCosCentric.com> <20060926165252.GA6697@lunn.ch>
Andrew Lunn wrote:
set_position() calculates the new absolute position and calls
cyg_stdio_lseek() which (with CYGPKG_FILEIO) is meant to be just a thin
veneer over lseek().
fseek() works fine. However flush_output_unlocked() does not call
set_position() before flushing out the buffer. It seems to assume the
file position is already in the correct position. The test case shows
its not. What has happened is a read is made which fills the buffer
and obviously leaves the file pointer after the end of the data just
read. There is when a write within this buffer, which is naturally
buffered. The test case then does a fseek() back to the beginning of
the file. This causes the buffer to be flushed, which just causes it
to be written out. There is no set_position() called to seek back to
the start of the buffer location in the file, so the data gets written
in the wrong place.
That doesn't seem like the right explanation. First of all, let's clarify
nomenclature: there are two "file positions" around, the stdio file
position which is where stdio will be reading from/writing to next, and
thus takes into account the buffer; and the underlying file pointer, which
is what is (or could be) passed to lseek, and is updated by read()s and
write()s as well. I'll call these the sfp and the ufp.
What you describe above is correct as written. Here's what should be happening:
After the first read, the sfp and the ufp will be updated. The sfp should
reflect what point was actually read to and likely corresponds to the start
of the buffer. The ufp corresponds to the end of the buffer. During the
write, the old read data in the buffer is emptied. After the write, there
may be stuff pending to be flushed in the buffer or not, but regardless sfp
should only be greater than ufp by the number of bytes in the buffer. The
fseek should cause the buffer to be flushed. flush_output_unlocked() will
write the buffer at the current ufp. After which sfp will equal ufp and
then the file is seeked.
So from looking at the code, and comparing to what should happen, I think
flush_output_unlocked is ok. Instead I think the problem is in
Cyg_StdioStream::write(). I think the problem is that we are nuking the
buffer but not accounting for the possibility of sfp != ufp. We need to
reset the ufp accordingly.
I think there is a problem in this area (but for a different reason) in
Cyg_StdioStream::read() as well.
The testcase as well as all the stdio and ramfs tests, pass with the
attached patch which I'll check in (easiest way for you to try it :-)). Let
me know whether you think it's ok.
It would be nice if in exchange, one of you could tidy up that testcase to
be a proper testcase suitable to be checked in.
Jifl
--
eCosCentric http://www.eCosCentric.com/ The eCos and RedBoot experts
------["The best things in life aren't things."]------ Opinions==mine
Index: src/common/stream.cxx
===================================================================
RCS file: /cvs/ecos/ecos/packages/language/c/libc/stdio/current/src/common/stream.cxx,v
retrieving revision 1.1.1.1
diff -u -5 -p -r1.1.1.1 stream.cxx
--- src/common/stream.cxx 4 Sep 2006 15:56:17 -0000 1.1.1.1
+++ src/common/stream.cxx 26 Sep 2006 18:44:33 -0000
@@ -7,10 +7,11 @@
//========================================================================
//####ECOSGPLCOPYRIGHTBEGIN####
// -------------------------------------------
// This file is part of eCos, the Embedded Configurable Operating System.
// Copyright (C) 1998, 1999, 2000, 2001, 2002 Red Hat, Inc.
+// Copyright (C) 2006 eCosCentric Limited
//
// eCos is free software; you can redistribute it and/or modify it under
// the terms of the GNU General Public License as published by the Free
// Software Foundation; either version 2 or (at your option) any later version.
//
@@ -384,12 +385,10 @@ Cyg_StdioStream::read( cyg_uint8 *user_b
*user_buffer = readbuf_char;
*bytes_read = 1;
flags.readbuf_char_in_use = false;
}
- position += *bytes_read;
-
// if we are unbuffered, we read as much as we can directly from the
// file system at this point.
//
// unless we do this, we could end up reading byte-by-byte from the filing system
@@ -403,10 +402,12 @@ Cyg_StdioStream::read( cyg_uint8 *user_b
len=buffer_length-*bytes_read;
read_err = cyg_stdio_read(my_device, user_buffer + *bytes_read, &len);
*bytes_read+=len;
}
+ position += *bytes_read;
+
unlock_me();
return read_err;
} // read()
@@ -613,12 +614,26 @@ Cyg_StdioStream::write( const cyg_uint8
unlock_me();
return EINVAL;
}
#ifdef CYGSEM_LIBC_STDIO_WANT_BUFFERED_IO
- if (flags.last_buffer_op_was_read == true)
+ if (flags.last_buffer_op_was_read == true) {
+#ifdef CYGPKG_LIBC_STDIO_FILEIO
+ if ( 0 != io_buf.get_buffer_space_used() )
+ {
+ off_t newpos = position;
+ io_buf.drain_buffer(); // nuke input bytes to prevent confusion
+ Cyg_ErrNo err = cyg_stdio_lseek( my_device, &newpos, SEEK_SET );
+ if (err) {
+ unlock_me();
+ return err;
+ }
+ }
+#else
io_buf.drain_buffer(); // nuke input bytes to prevent confusion
+#endif
+ }
flags.last_buffer_op_was_read = false;
if (!flags.buffering) {
#endif