Problem with storage write to pos 0

[espruino-discuss]

Posted at 2021-02-22 by JumJum

My idea was to define a file in storage and use it free like this:

var st = require("Storage");
var fn = "b";
st.write(fn,'X',0,20);
print(st.read(fn));
st.write(fn,[64,65,66],0);
print(st.read(fn));

This only works fine, if 2nd write does not use 0 for offset.
Otherwise, size is reduced to data written. In this case 3, and not 20 as defined in first write.

[gfwilliams]

Posted at 2021-02-22 by @gfwilliams

Ahh, that's interesting. What if you supply the file size in the argument as well? Does that help?

It might be because I'd expected that way of writing to be used mainly for logs or uploading from the Web IDE. Maybe I didn't consider that as a use-case.

It's a bit of a hack, but I guess for now you could just make the file one byte bigger and write to the address+1!

[espruino-discuss]

Posted at 2021-02-22 by JumJum

Supporting size again did not help.
I've a different hack ;-)

I'm using Storage to save some images data from GraphicsBuffer.
Creating the file was seperated from writing data.

//initialization of file storage
var st = require("Storage");
var fn = "b";
st.write(fn,'X',0,1024);
......
......
//write data to file
for(y = 0; y < height; y++){
  st.write(fn,data_for_one_line,y * width);  // write one line of graphics data
}

My hack is to remove write in initialization, and add size to write of first line.

[espruino-discuss]

Posted at 2021-02-23 by @fanoush

The Storage.write http://www.espruino.com/Reference#l_Storage_write says "You may also create a file and then populate data later as long as you don't try and overwrite data that already exists."

This is what you were doing as you supply one letter 'X' with first write. What if one sends null or empty string as data in first allocation call?

[espruino-discuss]

Posted at 2021-02-23 by JumJum

At least in my test, it did not matter, writing data or an empty string.
writing undefined or nothing gives an error, but, surprise,surprise, a file is created.
@fanoush, Data cannot be overwritten, thats a good point. I did not read this before.
BTW, try this:

st.write("a","",0,10);
=true
>st.read("a");
="\xFF\xFF\xFF\xFF\xFF\xFF\xFF\xFF\xFF\xFF"
>st.readArrayBuffer("a");
=new Uint8Array([255, 255, 255, 255, 255, 255, 255, 255, 255, 255]).buffer
>st.write("a","AB",0);
=true
>st.read("a")
="AB"
>st.readArrayBuffer("a");
=new Uint8Array([65, 66]).buffer
>st.write("a","CD",2);
Uncaught Error: Too much data for file size
 at line 1 col 20
st.write("a","CD",2);
                   ^
>st.write("a",[66,67],2);
Uncaught Error: Too much data for file size
 at line 1 col 23
st.write("a",[66,67],2);
                      ^
>st.read("a");
="AB"
> 

On the other hand, this works

>st.write("a","abcd",0,10);
=true
>st.readArrayBuffer("a");
=new Uint8Array([97, 98, 99, 100, 255, 255, 255, 255, 255, 255]).buffer
>st.write("a","efg",4);
=true
>st.readArrayBuffer("a");
=new Uint8Array([97, 98, 99, 100, 101, 102, 103, 255, 255, 255]).buffer
>st.read("a")
="abcdefg\xFF\xFF\xFF"
> 

Anyway, it works for me now ;-)

[espruino-discuss]

Posted at 2021-02-23 by @fanoush

yes, everything in first part is expected except line 7 st.write("a","AB",0);, that should overwrite first two 255 in array to be 'A','B','\xFF','\xFF',.... instead it will truncate the file and then the size is 2 so next lines fail

but the empty string seems to work, first byte is still 255 so uninitialized, so only writing to offset zero does not work correctly even if you don't initialize first byte

[espruino-discuss]

Posted at 2021-02-23 by @fanoush

this is interesting

>st.write("f",'',0,10)
=true
>st.read('f')
="\xFF\xFF\xFF\xFF\xFF\xFF\xFF\xFF\xFF\xFF"
>st.write("f",new Uint8Array([3,4]),2,10)
=true
>st.read('f')
="\xFF\xFF\3\4\xFF\xFF\xFF\xFF\xFF\xFF"
>st.write("f",new Uint8Array([5,6]),4,10)
=true
>st.read('f')
="\xFF\xFF\3\4\5\6\xFF\xFF\xFF\xFF"
>st.write("f",new Uint8Array([1]),1,10)
=true
>st.read('f')
="\xFF\1\3\4\5\6\xFF\xFF\xFF\xFF"
>st.write("f",new Uint8Array([0]),0,10)
=true
>st.read('f')
="\0\xFF\xFF\xFF\xFF\xFF\xFF\xFF\xFF\xFF"

you can write to different location and give total size in each call and data stays, you can even write to position 3,4 then 5,6 and then to 1(!, nice) but if you write to position 0 new file is created and rest is filled with \xFF

[espruino-discuss]

Posted at 2021-02-23 by @fanoush

the line 17 in my code above erases whole file becasue of this line https://github.com/espruino/Espruino/blob/ea8ba1c2ff9c9bfa1d19f5c5911818e0a43c7f4f/src/jsflash.c#L635
so you can write to other offsets randomly without erasing data but not to offset 0, that requires the file to be empty/erased/all \xFF

[gfwilliams]

Posted at 2021-02-23 by @gfwilliams

Yes - it's as you say, and I hinted above - Storage was designed for more of a 'linear' write - starting from 0 and moving on.

The issue is that when you have an Espruino C function that takes an integer, if there is no integer supplied the value is 0 - so Espruino has no way of knowing the difference between these two:

st.write("f",'xyz',0)
st.write("f",'xyz')

I guess it could be re-written to use a JsVar and check if the variable is undefined though - this just seems like a pretty small edge case

[espruino-discuss]

Posted at 2021-02-23 by @fanoush

I am giving the size too so it is actually like

st.write("f",'xyz',0,10)

while your example would be equal to

st.write("f",'xyz',0,0)

so it is possible to distinguish this, but I get the point.

Maybe allowing this when size is specified all the time (and is nonzero and matches) would make some sense to me - so only 4 parameter variant with nonzero size would support preallocated size use case. But maybe it can be said about three parameter variant too since once you start using offsets it is this case of using preallocated files. So having it as jsvar would make same sense and looks easier to use. So with that it would allow different semantics

• one time write possible = two parameters only
• multiple writes with preallocated size - 3 or 4 parameters - in this case writing to any offset including 0 should be possible (as long as data underneath currently written part is all FF).

[gfwilliams]

Posted at 2021-02-23 by @gfwilliams

Thinking back, I wonder whether the 'overwrite on 0' behaviour was actually to avoid problems with file uploads - since these start off just by writing address 0.

Suppose you upload the file Hello world 123 in two chunks, but now you upload the file Hello world 124 - you only find out the file has changed too late to create a new file, so the upload will fail.

That's amazingly common when uploading a JS app, when you might change some stuff and re-upload, keeping the file size the same.

So I guess if writing to address 0 we could read the old file to see if it had already been written at that address, and if so could re-allocate a new file.

However this seems amazingly hacky to cope with this one edge case, which you could solve just by having a single sacrificial byte at the start of the file

[espruino-discuss]

Posted at 2021-02-23 by @fanoush

I wonder whether the 'overwrite on 0' behaviour was actually to avoid problems with file uploads - since these start off just by writing address 0.

Why not to just call Storage.erase(file) from webide before calling Storage.write from offset 0 instead, if the intent was to erase the file before upload? Feels cleaner than relying on 'overwrite on 0' magic.

So I guess if writing to address 0 we could read the old file to see if it had already been written at that address, and if so could re-allocate a new file.

Don't get it. You need to know when to erase file with some existing data and when not? When starting to write same data from offset 0? Why? Would erasing it as mentioned above solve it or is this another case?

However this seems amazingly hacky to cope with this one edge case, which you could solve just by having a single sacrificial byte at the start of the file

Well, without understanding the context of using Storage to (re)upload files from Web IDE the 'amazingly hacky' is the way the Storage API works now regarding offset 0 ;-) Because without explaining it first the suggestion of 'having a single sacrificial byte at the start of the file' as a workaround feels exactly like that ;-)

Was just advocating for API with least surprises. Having writing to existing file to offset 0 truncate the file and using exactly same call with offset 1 not truncating it is confusing (=the sequence in post #7, lines 13 vs 17). However in reality one could prevent losing data like this by simply not writing to offset 0 after some data is already written before - that is even typical use case how to write to file.

More annoying on this is the need to write first block in special way - with data and total size together, or call it always like that - with 4 parameters including total size in each write call in a loop. Everything is possible to solve of course if you know that it works like this.

Becasue the 'naive' way of first preallocating/creating file with no data like write(f,"",0,size) and then writing in a loop starting from offset 0 write(f,data,offset) breaks (as seen in post #5 line 7).

So there are actually two issues - lost data, lost preallocated file. Both are caused by having offset and size as integer with default being 0 instead of JsVar. Optional offset could fix writing to location 0 (=create new file when not present, keep alone when present, even being zero), optional size would fix truncating size of previously preallocated file when size is not specified again when later writing first data to offset 0 (=check for matching size if present, keep alone if not).

Also maybe it could make the code cleaner as the semantics of single write of whole file vs repeated write to prealocated file with offset would be easier to distinguish from the way it is called. Lines 626 - 650 look quite complicated due to the need to solve/guess all those combinations
https://github.com/espruino/Espruino/blob/ea8ba1c2ff9c9bfa1d19f5c5911818e0a43c7f4f/src/jsflash.c#L626

[gfwilliams]

Posted at 2021-02-24 by @gfwilliams

Yes, this is less than ideal. However the API was only ever intended for Espruino tools, to allow them to upload large files in chunks - hence it's not super user friendly.

There are a lot of Espruino devices out there now, and the tools that talk to them (some of which are not web based so do not automatically upgrade) pretty much all use this API. I'm not breaking those tools and inconveniencing thousands of users for this (so forcing use of erase is not an option).

So I'm not sure that even changing offset to a JsVar would solve this (as it'd break existing tools too). I think there are two options:

• Check if the beginning of the existing file is erased when writing to 0. If it isn't, reallocate the file.
• Add a new function that can be used to write just part of a file - and maybe even allow it to write to bytes that have already been written.

Or, if you really want to do this, you can just use the direct flash write API which is easier and maybe more sensible anyway?

require("Storage").write("x","",0,2048);
var addr = E.getAddressOf(require("Storage").read("x"),true);
var f = require("Flash");

f.write("Hello world", addr);
// require("Storage").read("x") == "Hello world\xFF\xFF\xFF...."

Maybe the best thing is just to update the docs with this solution?

[espruino-discuss]

Posted at 2021-02-24 by @fanoush

Updating docs is best then I guess. Something like beware that writing to offset 0 creates new file so any data previously written is lost and file size is reset.

[espruino-discuss]

Posted at 2021-02-27 by tanepiper

Option 2 would be nice here - it seems a bit of a waste of cycles to have to read an entire file just to get the latest entry. The solution I've come up with is to read the file into an array, unshift with the latest row then re-write the file. With this case, I can make each line a valid JSON separated by a line break and it works reasonably efficiently, and I can just get the first line for the latest data.

[gfwilliams]

Posted at 2021-03-01 by @gfwilliams

Just to say, if you're rewriting the entire file, that will cause a whole new file to be written to flash (not just updating one bit). It's not an issue if you're only writing a few times, but you won't want to end up writing a lot!