emscripten-core / emscripten-core/emscripten
in memfs, canOwn flag causes entire file to be overwritten
- Dominant language
- C++
- Stars
- 27.6k
- Forks
- 3.6k
- Avg merge
- 1d 1h
- Merged PRs (30d)
- 105
Description
I was just looking at the implementation of memfs `write` and the `canOwn` flag seems to cause the file to be overwritten
https://github.com/emscripten-core/emscripten/blob/9b98d42be0c6111997a8317c38d3f97e520bf9dd/src/library_memfs.js#L287
```js
if (canOwn) {
#if ASSERTIONS
assert(position === 0, 'canOwn must imply no weird position inside the file');
#endif
node.contents = buffer.subarray(offset, offset + length);
node.usedBytes = length;
return length;
} else if (node.usedBytes === 0 && position === 0) { // If this is a simple first write to an empty file, do a fast set since we don't need to care about old data.
node.contents = buffer.slice(offset, offset + length);
node.usedBytes = length;
return length;
} else if (position + length <= node.usedBytes) { // Writing to an already allocated and used subrange of the file?
node.contents.set(buffer.subarray(offset, offset + length), position);
return length;
}
}
```
I think this should be instead:
```js
if (node.usedBytes === 0 && position === 0) { // If this is a simple first write to an empty file, do a fast set since we don't need to care about old data.
node.contents = canOwn ? buffer.subarray(offset, offset + length) : buffer.slice(offset, offset + length);
node.usedBytes = length;
return length;
} else if (position + length <= node.usedBytes) { // Writing to an already allocated and used subrange of the file?
node.contents.set(buffer.subarray(offset, offset + length), position);
return length;
}
}
```
Also, what if the argument is an `ArrayBuffer` instead of a view? Perhaps before the test for `buffer.subarray`, we should do:
```js
if(Object.prototype.toString.call(buffer) === "[object ArrayBuffer]"){
buffer = new Uint8Array(buffer);
}
```
and that way a raw `ArrayBuffer` will also get the faster treatment.
Contributor guide
Assessment
This issue has not been assessed yet.