* [ECOS] [eCos] a question about ROMFS @ 2001-07-16 0:47 jjtsai 2001-07-16 14:01 ` Jonathan Larmour 0 siblings, 1 reply; 6+ messages in thread From: jjtsai @ 2001-07-16 0:47 UTC (permalink / raw) To: ecos-discuss [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #1: Type: text/plain, Size: 655 bytes --] Hi,    I have a question about ROM files system.  [Question Description]    A fseek(,,SEEK_CUR) after fread() will cause inconsistency between "(CYG_StdioStream) real_stream.position"    and "fp->f_offset". See also stream.inl and rom_fs.c.        Here is my test program. See attchment: fseek_test.c .  [My modfication]  See attachment: stream_inl_patch.pat. Instruction about how to apply it: cd $(ECOS_REPOSITORY)/packages/language/c/libc/stdio/current/include patch -p0 < $(WHERE_THE_PATCH_IS)/stream_inl_patch.pat regards, Jang-Jer fseek_test.c stream_inl_patch.pat [-- Attachment #2: fseek_test.c --] [-- Type: text/x-c, Size: 1464 bytes --] #include <unistd.h> #include <fcntl.h> #include <stdio.h> #include <cyg/fileio/fileio.h> #include <pkgconf/fs_rom.h> // Address of ROMFS //========================================================================== MTAB_ENTRY( romfs_mte1, "/", "romfs", "", (CYG_ADDRWORD) CYGNUM_FS_ROM_BASE_ADDRESS ); //========================================================================== // main int main( int argc, char **argv ) { FILE *fd; int err; int i; int length; char s[1000]; struct stat sbuf; stat("/HelloWorld.jar", &sbuf); printf("leng=%d\n",sbuf.st_size); if( (fd=fopen("/HelloWorld.jar","rw")) == NULL) printf("fopen error\n"); else { length=0; do{ err=ftell(fd); printf("ftell=%d\n",err); //ftell should return 0; if( (err=fseek(fd, 195, SEEK_SET))<0) printf("fseek=%d\n",err); err=ftell(fd); printf("ftell=%d\n",err); //ftell should return 195 if( (err=fread((s+length),sizeof(char), 30, fd))!=30) printf("fread=%d\n",err); err=ftell(fd); printf("ftell=%d\n",err); //ftell should return 225 if( (err=fseek(fd, 0, SEEK_CUR))<0) printf("fseek=%d\n",err); err=ftell(fd); printf("ftell=%d\n",err); //ftell should return 225 if( (err=fseek(fd, 1, SEEK_CUR))<0) printf("fseek=%d\n",err); err=ftell(fd); printf("ftell=%d\n",err); //ftell should return 226 } while(0); fclose(fd); } return 0; } [-- Attachment #3: stream_inl_patch.pat --] [-- Type: text/x-diff, Size: 566 bytes --] --- stream.inl Sat Apr 7 01:20:40 2001 +++ stream.inl.my Mon Jul 16 14:37:34 2001 @@ -406,7 +406,16 @@ if (!lock_me()) return EBADF; // assume file is now invalid - err = cyg_stdio_lseek( my_device, &newpos, whence ); + //add jjt + if( whence == SEEK_CUR ){ + newpos = position + pos; + err = cyg_stdio_lseek( my_device, &newpos, SEEK_SET); + } + else { + err = cyg_stdio_lseek( my_device, &newpos, whence ); + } + //end jjt + //err = cyg_stdio_lseek( my_device, &newpos, whence ); //del jjt if( err == ENOERR ) { ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [ECOS] [eCos] a question about ROMFS 2001-07-16 0:47 [ECOS] [eCos] a question about ROMFS jjtsai @ 2001-07-16 14:01 ` Jonathan Larmour 2001-07-17 0:11 ` jjtsai 0 siblings, 1 reply; 6+ messages in thread From: Jonathan Larmour @ 2001-07-16 14:01 UTC (permalink / raw) To: jjtsai; +Cc: ecos-discuss > jjtsai wrote: > > Hi, > I have a question about ROM files system. > > [Question Description] > A fseek(,,SEEK_CUR) after fread() will cause inconsistency between > "(CYG_StdioStream) real_stream.position" > and "fp->f_offset". See also stream.inl and rom_fs.c. I believe I understand the problem: the position required by lseek differs from the stream position, potentially by the number of bytes in the buffer. In that case your patch is not quite complete to address this problem: what if you are moving forward 1 byte, ie. fseek(,1, SEEK_CUR), *but* the data is already in the buffer. For a true stream rather than a file, we should not flush the input buffer. I'll see if I can work on a better patch. Jifl -- Red Hat, Rustat House, Clifton Road, Cambridge, UK. Tel: +44 (1223) 271062 Maybe this world is another planet's Hell -Aldous Huxley || Opinions==mine Come to the Red Hat TechWorld open source conference in Brussels! Keynotes, techie talks and exhibitions http://www.redhat-techworld.com/ ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [ECOS] [eCos] a question about ROMFS 2001-07-16 14:01 ` Jonathan Larmour @ 2001-07-17 0:11 ` jjtsai 2001-07-17 13:09 ` Jonathan Larmour 0 siblings, 1 reply; 6+ messages in thread From: jjtsai @ 2001-07-17 0:11 UTC (permalink / raw) To: Jonathan Larmour; +Cc: ecos-discuss ----- Original Message ----- From: Jonathan Larmour <jlarmour@redhat.com> To: jjtsai <jjtsai@itri.org.tw> Cc: <ecos-discuss@sources.redhat.com> Sent: Tuesday, July 17, 2001 5:01 AM Subject: Re: [ECOS] [eCos] a question about ROMFS > > [Question Description] > > A fseek(,,SEEK_CUR) after fread() will cause inconsistency between > > "(CYG_StdioStream) real_stream.position" > > and "fp->f_offset". See also stream.inl and rom_fs.c. > > I believe I understand the problem: the position required by lseek differs > from the stream position, potentially by the number of bytes in the buffer. > In that case your patch is not quite complete to address this problem: what > if you are moving forward 1 byte, ie. fseek(,1, SEEK_CUR), *but* the data > is already in the buffer. For a true stream rather than a file, we should > not flush the input buffer. Yes, you do understand my question and my solution. :p > I'll see if I can work on a better patch. That would be great! best regards, JJ ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [ECOS] [eCos] a question about ROMFS 2001-07-17 0:11 ` jjtsai @ 2001-07-17 13:09 ` Jonathan Larmour 2001-07-18 0:07 ` jjtsai 0 siblings, 1 reply; 6+ messages in thread From: Jonathan Larmour @ 2001-07-17 13:09 UTC (permalink / raw) To: jjtsai; +Cc: ecos-discuss jjtsai wrote: > > I'll see if I can work on a better patch. > That would be great! Can you try the attached patch for me please? I haven't even tried compiling it yet nevermind testing it, but I was hoping you could do that instead :-). Let me know how it goes. Then I'll check it in. Jifl -- Red Hat, Rustat House, Clifton Road, Cambridge, UK. Tel: +44 (1223) 271062 Maybe this world is another planet's Hell -Aldous Huxley || Opinions==mine Come to the Red Hat TechWorld open source conference in Brussels! Keynotes, techie talks and exhibitions http://www.redhat-techworld.com/ Index: include/stream.inl =================================================================== RCS file: /home/cvs/ecc/ecc/language/c/libc/stdio/current/include/stream.inl,v retrieving revision 1.3 diff -u -5 -p -r1.3 stream.inl --- include/stream.inl 2001/03/15 20:30:22 1.3 +++ include/stream.inl 2001/07/17 20:05:43 @@ -361,19 +361,19 @@ Cyg_StdioStream::get_position( fpos_t *p // set absolute position inline Cyg_ErrNo Cyg_StdioStream::set_position( fpos_t pos, int whence ) { + CYG_ASSERTCLASS( this, "Stream object is not a valid stream!" ); + #ifndef CYGPKG_LIBC_STDIO_FILEIO // this is currently a workaround until we have real files // this will be corrected when we decide the true filesystem interface Cyg_ErrNo err; cyg_uint8 c; - CYG_ASSERTCLASS( this, "Stream object is not a valid stream!" ); - if ((whence != SEEK_CUR) || pos < 0) return ENOSYS; if (!lock_me()) return EBADF; // assume file is now invalid @@ -396,18 +396,46 @@ Cyg_StdioStream::set_position( fpos_t po return ENOERR; #else - Cyg_ErrNo err; - off_t newpos = pos; - - CYG_ASSERTCLASS( this, "Stream object is not a valid stream!" ); - if (!lock_me()) return EBADF; // assume file is now invalid + if ( whence != SEEK_END ) { + cyg_ucount32 bytesavail = bytes_available_to_read(); + off_t abspos = (whence == SEEK_CUR) ? position + pos : pos; + off_t posdiff = abspos - position; + + if ( bytesavail > posdiff ) { + // can just "seek" within the existing buffer +#ifdef CYGFUN_LIBC_STDIO_ungetc + if (flags.unread_char_buf_in_use) { + flags.unread_char_buf_in_use = false; + posdiff--; + } +#endif +#ifdef CYGSEM_LIBC_STDIO_WANT_BUFFERED_IO + if (posdiff>0 && flags.buffering) { + io_buf.set_bytes_read(posdiff); + posdiff=0; + } else +#endif + if (posdiff>0 && flags.readbuf_char_in_use) { + flags.readbuf_char_in_use = false; + posdiff--; + } + CYG_ASSERT(posdiff==0, "Failed to seek within buffer correctly"); + + position = abspos; + unlock_me(); + return ENOERR; + } + } + + Cyg_ErrNo err; + off_t newpos = pos; err = cyg_stdio_lseek( my_device, &newpos, whence ); if( err == ENOERR ) { // Clean out the buffer. Flush output if any present, @@ -420,14 +448,14 @@ Cyg_StdioStream::set_position( fpos_t po flags.unread_char_buf_in_use = false; #endif // Clear EOF indicator. flags.at_eof = false; - } - - if( err == ENOERR ) + + // update stream pos position = newpos; + } unlock_me(); return err; #endif ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [ECOS] [eCos] a question about ROMFS 2001-07-17 13:09 ` Jonathan Larmour @ 2001-07-18 0:07 ` jjtsai 2001-07-19 23:42 ` Jonathan Larmour 0 siblings, 1 reply; 6+ messages in thread From: jjtsai @ 2001-07-18 0:07 UTC (permalink / raw) To: Jonathan Larmour; +Cc: ecos-discuss [-- Attachment #1: Type: text/plain, Size: 1138 bytes --] ----- Original Message ----- From: Jonathan Larmour <jlarmour@redhat.com> To: jjtsai <jjtsai@itri.org.tw> Cc: <ecos-discuss@sources.redhat.com> Sent: Wednesday, July 18, 2001 4:09 AM Subject: Re: [ECOS] [eCos] a question about ROMFS > jjtsai wrote: > > > I'll see if I can work on a better patch. > > That would be great! > > Can you try the attached patch for me please? I haven't even tried > compiling it yet nevermind testing it, but I was hoping you could do that > instead :-). Let me know how it goes. Then I'll check it in. Your attached patch still has problems. Test program (fseek.c) is attached. I do a little modification on the stream.inl (based on your patch) and romfs.c. It works well in my test program. Patch files are attached. Have a try please. I am afraid if I make any mistakes. Instruction about how to apply the patch files: cd $(ECOS_REPOSITORY)/packages/language/c/libc/stdio/current/include patch -p0 < $(WHERE_THE_PATCH_IS)/stream_inl.pat (This is the patch to the original file.) cd $(ECOS_REPOSITORY)/packages/fs/rom/current/src patch -p0 < $(WHERE_THE_PATCH_IS)/romfs_c.pat best regards, JJ [-- Attachment #2: fseek.c --] [-- Type: text/x-c, Size: 1773 bytes --] #include <unistd.h> #include <fcntl.h> #include <stdio.h> #include <cyg/fileio/fileio.h> #include <pkgconf/fs_rom.h> // Address of ROMFS //========================================================================== MTAB_ENTRY( romfs_mte1, "/", "romfs", "", (CYG_ADDRWORD) CYGNUM_FS_ROM_BASE_ADDRESS ); //========================================================================== // main int main( int argc, char **argv ) { FILE *fd; int err; int i; int length; char s[1000]; struct stat sbuf; stat("/HelloWorld.jar", &sbuf); printf("leng=%d\n",sbuf.st_size); if( (fd=fopen("/HelloWorld.jar","rw")) == NULL) printf("fopen error\n"); else { length=0; do{ err=ftell(fd); printf("ftell=%d\n",err); //ftell should return 0; if( (err=fseek(fd, 195, SEEK_SET))<0) printf("fseek=%d\n",err); err=ftell(fd); printf("ftell=%d\n",err); //ftell should return 195 if( (err=fread((s+length),sizeof(char), 30, fd))!=30) printf("fread=%d\n",err); err=ftell(fd); printf("ftell=%d\n",err); //ftell should return 225 if( (err=fseek(fd, 1, SEEK_CUR))<0) printf("fseek=%d\n",err); err=ftell(fd); printf("ftell=%d\n",err); //ftell should return 226 if( (err=fread((s+length),sizeof(char), 30, fd))!=30) printf("fread=%d\n",err); err=ftell(fd); printf("ftell=%d\n",err); //ftell should return 256 if( (err=fseek(fd, 400, SEEK_CUR))<0) printf("fseek=%d\n",err); err=ftell(fd); printf("ftell=%d\n",err); //ftell should return 656 if( (err=fread((s+length),sizeof(char), 30, fd))!=30) printf("fread=%d\n",err); err=ftell(fd); printf("ftell=%d\n",err); //ftell should return 686 } while(0); fclose(fd); } return 0; } [-- Attachment #3: romfs_c.pat --] [-- Type: text/x-diff, Size: 518 bytes --] --- romfs.c Wed Jul 18 14:50:23 2001 +++ romfs.c.new Wed Jul 18 14:28:15 2001 @@ -912,10 +912,16 @@ { case SEEK_SET: // Pos is already where we want to be. + fp->f_offset= *apos; //add jjt break; case SEEK_CUR: // Add pos to current offset. + // comment jjt + // the following line should NEVER be executed + // because it will cause the inconsistency between + // fp->f_offset and real_stream.position. + // end comment jjt pos += fp->f_offset; break; [-- Attachment #4: stream_inl.pat --] [-- Type: text/x-diff, Size: 2666 bytes --] --- stream.inl Wed Jul 18 14:45:00 2001 +++ stream.inl.new Wed Jul 18 14:44:09 2001 @@ -363,6 +363,8 @@ inline Cyg_ErrNo Cyg_StdioStream::set_position( fpos_t pos, int whence ) { + CYG_ASSERTCLASS( this, "Stream object is not a valid stream!" ); + #ifndef CYGPKG_LIBC_STDIO_FILEIO // this is currently a workaround until we have real files // this will be corrected when we decide the true filesystem interface @@ -370,8 +372,6 @@ Cyg_ErrNo err; cyg_uint8 c; - CYG_ASSERTCLASS( this, "Stream object is not a valid stream!" ); - if ((whence != SEEK_CUR) || pos < 0) return ENOSYS; @@ -398,15 +398,54 @@ #else - Cyg_ErrNo err; - off_t newpos = pos; - - CYG_ASSERTCLASS( this, "Stream object is not a valid stream!" ); - if (!lock_me()) return EBADF; // assume file is now invalid - err = cyg_stdio_lseek( my_device, &newpos, whence ); + if ( whence != SEEK_END ) { + cyg_ucount32 bytesavail = bytes_available_to_read(); + off_t abspos = (whence == SEEK_CUR) ? position + pos : pos; + off_t posdiff = abspos - position; + + if ( bytesavail > posdiff ) { + // can just "seek" within the existing buffer +#ifdef CYGFUN_LIBC_STDIO_ungetc + if (flags.unread_char_buf_in_use) { + flags.unread_char_buf_in_use = false; + posdiff--; + } +#endif +#ifdef CYGSEM_LIBC_STDIO_WANT_BUFFERED_IO + if (posdiff>0 && flags.buffering) { + io_buf.set_bytes_read(posdiff); + posdiff=0; + } else +#endif + if (posdiff>0 && flags.readbuf_char_in_use) { + flags.readbuf_char_in_use = false; + posdiff--; + } + CYG_ASSERT(posdiff==0, "Failed to seek within buffer correctly"); + + position = abspos; + unlock_me(); + return ENOERR; + } // endif (bytesavail > posdiff) + } //endif (whence != SEEK_END) + + Cyg_ErrNo err; + off_t newpos=pos; + + //add jjt + if( whence == SEEK_CUR ){ + newpos = position + pos; + err = cyg_stdio_lseek( my_device, &newpos, SEEK_SET); + } + else { + err = cyg_stdio_lseek( my_device, &newpos, whence ); + } + //end jjt + //err = cyg_stdio_lseek( my_device, &newpos, whence ); //del jjt + if( err == ENOERR ) { @@ -422,10 +461,10 @@ // Clear EOF indicator. flags.at_eof = false; - } - - if( err == ENOERR ) + + // update stream pos position = newpos; + } unlock_me(); ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [ECOS] [eCos] a question about ROMFS 2001-07-18 0:07 ` jjtsai @ 2001-07-19 23:42 ` Jonathan Larmour 0 siblings, 0 replies; 6+ messages in thread From: Jonathan Larmour @ 2001-07-19 23:42 UTC (permalink / raw) To: jjtsai; +Cc: ecos-discuss jjtsai wrote: > > ----- Original Message ----- > From: Jonathan Larmour <jlarmour@redhat.com> > To: jjtsai <jjtsai@itri.org.tw> > Cc: <ecos-discuss@sources.redhat.com> > Sent: Wednesday, July 18, 2001 4:09 AM > Subject: Re: [ECOS] [eCos] a question about ROMFS > > > jjtsai wrote: > > > > I'll see if I can work on a better patch. > > > That would be great! > > > > Can you try the attached patch for me please? I haven't even tried > > compiling it yet nevermind testing it, but I was hoping you could do that > > instead :-). Let me know how it goes. Then I'll check it in. > Your attached patch still has problems. Test program (fseek.c) is attached. > > I do a little modification on the stream.inl (based on your patch) and > romfs.c. > It works well in my test program. Patch files are attached. > > Have a try please. I am afraid if I make any mistakes. > > Instruction about how to apply the patch files: > > cd $(ECOS_REPOSITORY)/packages/language/c/libc/stdio/current/include > patch -p0 < $(WHERE_THE_PATCH_IS)/stream_inl.pat > (This is the patch to the original file.) Oops, I was so caught up in trying to seek within the buffer, I forgot about the original problem :). I'm going to update anon CVS with this later today. You can try it from there to make sure it's okay. I made a slight variation on your change, just for efficiency and code size. > cd $(ECOS_REPOSITORY)/packages/fs/rom/current/src > patch -p0 < $(WHERE_THE_PATCH_IS)/romfs_c.pat I don't get this one. Your change doesn't appear to do anything because further in dev_fo_lseek there is: // All OK, set fp offset and return new position. *apos = fp->f_offset = pos; And your comment for the SEEK_CUR case: are you saying that pos shouldn't be changed? If so, can you explain in more detail why? Jifl -- Red Hat, Rustat House, Clifton Road, Cambridge, UK. Tel: +44 (1223) 271062 Maybe this world is another planet's Hell -Aldous Huxley || Opinions==mine Come to the Red Hat TechWorld open source conference in Brussels! Keynotes, techie talks and exhibitions http://www.redhat-techworld.com/ ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2001-07-19 23:42 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2001-07-16 0:47 [ECOS] [eCos] a question about ROMFS jjtsai 2001-07-16 14:01 ` Jonathan Larmour 2001-07-17 0:11 ` jjtsai 2001-07-17 13:09 ` Jonathan Larmour 2001-07-18 0:07 ` jjtsai 2001-07-19 23:42 ` Jonathan Larmour
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).