itsanjan / arduino

Automatically exported from code.google.com/p/arduino
Other
0 stars 0 forks source link

Bug in SD library? File.available() returns int iso long #571

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago

Seen @ http://arduino.cc/forum/index.php/topic,66929.0.html

From File.cpp:

int File::available() {
  return size() - position();
}

uint32_t File::position() {
  return SD.file.curPosition();
}

uint32_t File::size() {
  return SD.file.fileSize();
}

File.available() returns an int but it should return uint32_t;

FOr big files one can get negative output when calling File.available() which 
is not expected.

IDE22, WIN7, 2009& UNO

Original issue reported on code.google.com by rob.till...@gmail.com on 19 Jul 2011 at 3:21

GoogleCodeExporter commented 9 years ago
Fix is easy, just change returntype to uint32_t
Rob

Original comment by rob.till...@gmail.com on 19 Jul 2011 at 3:22

GoogleCodeExporter commented 9 years ago
Thank you for the above fix, it solved a problem I had trying to play long 
files to an MP3 shield. Works a treat now!

This does seem like quite a sneaky bug, is there any way to raise the priority 
for a fix to the main code line?

Kindest regards..

Original comment by bogdown...@gmail.com on 16 Nov 2011 at 11:59

GoogleCodeExporter commented 9 years ago
This should be fixed in the Arduino 1.0 release candidates: 
https://github.com/arduino/Arduino/blob/master/libraries/SD/File.cpp (by simply 
capping the return value of available() so it doesn't go negative).

Original comment by dmel...@gmail.com on 16 Nov 2011 at 2:14

GoogleCodeExporter commented 9 years ago
Do not agree,

File::size() returns an uint32_t and position returns an uint32_t and the 
position can by definition only be equal or smaller than the size. So there is 
no need to mask the sign bit, as in unsigned ints there exists no sign bit. 

The proposed code for available() in 1.0 does not make a difference between the 
return of 0 if there is "no _file" and the case when the file is completely 
read, as then available() should also return 0. 

I propose to change the 1.0 code to:

uint32_t File::available() {
  return size() - position();
}

This keeps it simple and straightforward. If the file does not exist both 
size() and position should return 0 so available returns 0 and that is correct 
(at least not wrong)

BTW the code for position() has a BUG in it 

uint32_t File::position() {
  if (! _file) return -1;  <<<< the function returns an uint32_t so -1 is wrong....
  return _file->curPosition();
}

maybe add the following so people can check if the existance of the file.

boolean File::exists() 
{
  return _file;
}

my 2 cents,
Rob

Original comment by rob.till...@gmail.com on 17 Nov 2011 at 7:16