Bug in fsrw

rokicki

ElectricAxe actually found a major bug in fsrw.

This is in all versions of fsrw released so far.

It only exhibits if you mix pputc() and pwrite().

I am shocked this bug did not get picked up in all the many hours of random testing I did.

The bug is as follows. If a pwrite *just* fills the buffer, without flushing it, and then a
pputc() is called, the 513th byte in the buffer will be smashed.

I am tempted to do an emergency rerelease of fsrw, but I'm also a bit worried that any
such haste may introduce new bugs.

I do not yet have a fix.

As long as you don't mix pputc() and pwrite(), you should be fine.

Thank you, ElectricAxe, for creating the program that found this.

I'll probably be releasing a new version (with this fix and another
enhancement by mirror, and the MIT license) tomorrow night.

Post Edited (rokicki) : 9/24/2008 5:21:18 AM GMT

Mike Green

This bug does not occur in FemtoBasic and its derivatives because pwrite() is only used by the COPY statement
and pputc() is only used by the FILE = statement and there's no mixing of pwrite() and pputc() internally.

Thanks ElectricAye and Tomas.

Post Edited (Mike Green) : 9/24/2008 5:36:45 AM GMT

Carl Jacobs

This bug does not occur in JDForth due to the refactoring that took place when it was translated.

fs.Write and fs.PutC may be intermixed at will as both call the word WritePrepare which ensures that there is space in the buffer before the write commences.

In the specific instance noted, fs.Write would just fill the buffer without flushing it; when fs.PutC is called it checks and flushes the buffer just before writing it's single byte.

▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔
Carl Jacobs


JDForth - Forth to Spin Compiler http://www.jacobsdesign.com.au/software/jdforth/jdforth.php
Includes: FAT16 support for SD cards. Bit-bash Serial at 2M baud. 32-bit floating point maths.·Fib(28) in 0.86 seconds. ~3x faster than spin, ~40% larger than spin.

rokicki

Okay, I believe I've fixed the problem (and implemented a few other improvements). The new fsrw is at

http://tomas.rokicki.com/fat16work/

I'm not going to upload it to the object exchange quite yet. I'm looking for volunteers who are willing to
download it and try to break it. (And if on the way we get some nice examples, that's cool too.)

It passes my thrash tests (but so did the earlier version, somehow), and it passes my cutdown of
ElectricAxe's test (that failed it), and it passes the other small programs I have written.

The other changes are:

1. MIT license

2. If the command byte is not one of the expected ones, it ignores it (unlike before, where it
would assume it was a write command). Thanks to mirror for suggesting that.

ElectricAye

rokicki,

That's GREAT! Thank you for chasing down this problem! Honestly, you have lifted my spirits today. I was beginning to think my programming skills were just plain hopeless. It will probably take me a day or two to try out your new version of software but I'll be sure to give you some feedback once I do.

THANKS!


Mark

PS. And thanks to Mike and Carl for noting other options!

ElectricAye

rokicki,

I just ran my bloated Spin program using your updated fsrw, and everything looks BEE-YOO-TIF-FULL!

I can't thank you enough for fixing this problem. You really saved my gluteus maximus!


bless you!

Mark

rokicki

It is I who thank you; it was *my* bug. And I'm sure, going forward, other people will be very
happy too, to not trip over this problem.

StefanL38

Hello to all the propeller-newbees,

My opinion: newbees and amateurs are the most professional softwaretesters as they combine things a professional programmer would never think of.
And this intensivly testing is the hardest test-procedure you can do.

So all newbees are welcome to test the objects. If errors occure you will learn a lot of from the errors and/or some times
hidden bugs will be found this way.

best regards

Stefan

Pyrotom

Tom - have you considered incorporating the changes Harrison made in fsrw_shared, which allow the SD card to be mounted from a high level object and then used from lower level objects?

rokicki

I've considered it, but I'm not sure it's a slam-dunk; there are people who mount several cards,
and they need separate fsrw objects for concurrent access, so I'm not quite willing yet to say
that fsrw has to be always global.

I'm (still) trying to come up with a mechanism for managing enhancements. Code space is
extremely limited on the prop, and I would like to add:

1. Seek
2. R+W on the same file
3. Directories
4. Formatting
5. Multiple files open at once
6. Shared access and/or unshared access
7. SDHC

but if I add all of these, the object will be four or five times its current size and won't fit with
many user programs.

If I can figure out a way to manage combinations of features, without leaving the existing
Spin compiler, I could start working on some of these.

There's no way I can support all combinations of features as separate source code bases.

At the moment, fsrw attempts to give the minimal functionality that is generally useful,
while specifically omitting functionality that is not needed by most people, primarily due to
the 32K memory limit.

Baggers

rokicki, what about if you put them all in, but have block comments around the seperate bits, so people can remove the specific parts of the code they don't need? that way, it keeps it all in one managable object, rather than you having to manage multiple versions of FSRW [noparse]:)[/noparse]

▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔
http://www.propgfx.co.uk/forum/·home of the PropGFX Lite

·

rokicki

That's a possibility, but it's a bit ugly.

Frankly, if only the prop tool would excise unused routines, that would go a long way towards solving the problem.
No sense wasting memory for subroutines that are never called.

Mike Green

Having started some work on some of these enhancements, my experience is that seek doesn't seem to take too much in terms of code, neither does read and write on the same file. Multiple files open at once requires a sector buffer (512 bytes) and a copy of all the file-specific variables per file (a separate instance of the object). Read-only directories doesn't seem too hard as long as you move up and down the directory tree rather than requiring FSRW to parse and process a file path itself.

Formatting and directory maintenance can be done using a utility program or a utility object that also exports the basic FSRW functions.

Baggers

rokicki, I know it's a bit ugly, and unless mpark puts a conditional compilation section into his home-spun-compiler, then I'd say it's tidier than having multiple skew's for you to maintain [noparse]:)[/noparse]

▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔
http://www.propgfx.co.uk/forum/·home of the PropGFX Lite

·

hippy

Maybe fsrw needs delivering as two parts; the .Spin file and a utility which allows the actual .Spin to be created fro that with only what's required by the developer, conditional compilation as an external process.

Large memory footprint shouldn't be a problem while developing and testing fsrw, nor for those just taking the object to use as is, only those who need to tailor it to use less memory would need to run the 'configuration utility' and use the sub-set fsrw object created.

Not an ideal solution but does get round when the PropTool fails to measure up to what one expects from a professional development environment. I've run into similar problems of trying to maintain a single code base with configurable usage options and it's not easy.

mpark

Baggers said...
rokicki, I know it's a bit ugly, and unless mpark puts a conditional compilation section into his home-spun-compiler, then I'd say it's tidier than having multiple skew's for you to maintain [noparse]:)[/noparse]

Just fyi, Homespun now·supports conditional compilation.