FAQFAQ   SearchSearch   MemberlistMemberlist   UsergroupsUsergroups   RegisterRegister 
 ProfileProfile   Log in to check your private messagesLog in to check your private messages   Log inLog in 

[Bug] Deadly side effect of File.size()

 
Post new topic   Reply to topic     Forum Index -> Indigo
View previous topic :: View next topic  
Author Message
smjg



Joined: 29 Sep 2004
Posts: 41
PostPosted: Fri Aug 12, 2005 10:06 am    Post subject: [Bug] Deadly side effect of File.size() Reply with quote
Using DMD 0.129, Using Windows 98SE.

It took me a while to isolate a bug. Both File.size() and File.flush() call f_fflush. On a file that is open for ReadOnly or ReadWrite, this has the side effect of moving the file pointer to the end of the file, if a read operation has taken place. I think what actually happens is that it clears out the underlying C FILE's internal buffer, hence anything in this buffer waiting to be read is lost.

Consequently, bytesAvailable becomes practically useless.

It's a shame the C library didn't provide separate functions to flush input and output. Meanwhile, I suppose we can ask ourselves: is there any point flushing here at all?
  • Do we need the value returned by size() to be up to date with pending write operations?
  • If so, is there a better way of doing it? (I'm guessing making size() return the maximum of the actual size and pos() would do it, but haven't had time to test.)
Anyway, here's some code that shows the problem:
Code:

import indigo.io.file;
import indigo.io.datastream;
import std.stdio;

void main() {
   auto File inputFile = new File("input.txt");

   inputFile.open(IODevice.ReadOnly);

   auto DataStream ds = new DataStream(inputFile);
   char c;

   while (!inputFile.atEnd) {
      writefln("Available: ?d", inputFile.bytesAvailable());
      ds >> c;
      if (c == 0xFF) {
         writefln("ff");
      } else {
         writefln("?02x: ?s", c, c);
      }
   }
}
Output (with behaviour of atEnd() reverted to the original):

Available: 46
54: T
Available: 0
Error: Unexpected end of data stream.

Stewart.
Back to top
View user's profile Send private message
uwe



Joined: 05 Apr 2005
Posts: 34
Location: Stuttgart, Germany
PostPosted: Sat Aug 13, 2005 2:57 am    Post subject: Good catch Reply with quote
After reading File.size() i cannot even tell how in the world i got the idea that the file needs to be flushed there. Perhaps to get pending write operations done. Perhaps this is just useless and we could call fstat() instantly.

Or perhaps the code should be changed to something like this:

Code:

long curPos = ftell(m_file);
long endPos = fseek(m_file, 0, SEEK_END);
fseek(m_file, curPos, SEEK_SET);


But i don't know if this will break pending/future read/write operations. Perhaps you could test that? That would be very nice.

Ciao
uwe
Back to top
View user's profile Send private message
smjg



Joined: 29 Sep 2004
Posts: 41
PostPosted: Sat Aug 13, 2005 3:54 am    Post subject: Re: Good catch Reply with quote
uwe wrote:
After reading File.size() i cannot even tell how in the world i got the idea that the file needs to be flushed there. Perhaps to get pending write operations done. Perhaps this is just useless and we could call fstat() instantly.

Or perhaps the code should be changed to something like this:

Code:
long curPos = ftell(m_file);
long endPos = fseek(m_file, 0, SEEK_END);
fseek(m_file, curPos, SEEK_SET);


But i don't know if this will break pending/future read/write operations. Perhaps you could test that? That would be very nice.

fseek doesn't return the resulting position, only a success/failure value. You have to call ftell again to find out where it ended up.

I now have this similar code:
Code:
long curPos = f_ftell(m_file);
if (f_fseek(m_file, 0, fc_fseekend) != 0)
  throw new IODeviceException("File position could not be set:\n" ~ f_errorstring()); // TODO i18n
long endPos = f_ftell(m_file);
if (f_fseek(m_file, curPos, fc_fseekset) != 0)
  throw new IODeviceException("File position could not be set:\n" ~ f_errorstring()); // TODO i18n
return endPos;

which seems to work at the moment. But there are problems with both ideas:
  • This won't work on files greater than 4GB, on platforms where the file API is only 32-bit. (Do such platforms allow such huge files?)
  • The other solution (just call f_fstat) could lead to subtraction overflow in bytesAvailable if there are pending write operations.

But your idea gives me an idea for another (untested) way of implementing atEnd:
Code:
long curPos = f_ftell(m_file);
f_fseek(m_file, 0, fc_seekend);
if (f_ftell(m_file) == curPos) return true;
f_fseek(m_file, 0, fc_seekset);
return false;

Stewart.
Back to top
View user's profile Send private message
uwe



Joined: 05 Apr 2005
Posts: 34
Location: Stuttgart, Germany
PostPosted: Sat Aug 13, 2005 11:45 pm    Post subject: Reply with quote
Please look into the current stewart.tar.gz. I have deleted the fileengine module, and File now uses the indigo.posix.* modules. If you make changes to the old file module, we have to redo them.

In the indigo.posix.* modules i have declared 64-bit access functions, which can transparently replace the older functions if compiled with version "Indigo_File64Bit". But you are right, the current Windows library shipping with D does not define these functions (ftell64 and fseek64). They are also missing from the stat() API. But the good news is that this is the reason i wrote the POSIX wrapper. All the OS dependence is concentrated into this subpackage, thus we can write our own Windows implementation as soon as we have time to :)

Your idea for atEnd() is nice. If the size() implementation works and does not interfere with reading/writing operations, this should do as well.

Ciao
uwe
Back to top
View user's profile Send private message
Display posts from previous:   
Post new topic   Reply to topic     Forum Index -> Indigo All times are GMT - 6 Hours
Page 1 of 1

 
Jump to:  
You cannot post new topics in this forum
You cannot reply to topics in this forum
You cannot edit your posts in this forum
You cannot delete your posts in this forum
You cannot vote in polls in this forum


Powered by phpBB © 2001, 2005 phpBB Group