s-macke / jor1k

Online OR1K Emulator running Linux
http://jor1k.com
BSD 2-Clause "Simplified" License
1.73k stars 193 forks source link

File sync not working for existing profile #141

Closed elixx closed 5 years ago

elixx commented 5 years ago

I made a fix for the file sync process -- I encountered a bug where inode.mtime was undefined in the JavaScript console if a user profile was loaded via /?user=blah and the tarball for blah already exists. No updated tarball would get written.

This fixes the undefined variable:

diff --git a/demos/jor1k-worker-min.js b/demos/jor1k-worker-min.js
index f7ea3f4..ceeca69 100644
--- a/demos/jor1k-worker-min.js
+++ b/demos/jor1k-worker-min.js
@@ -6373,6 +6373,7 @@ TAR.prototype.Pack = function(path) {
         WriteStringToBinary((inode.mode&0xFFF).toString(8), buffer, offset+100, 8); // mode
         WriteStringToBinary(inode.uid.toString(8), buffer, offset+108, 8); // uid
         WriteStringToBinary(inode.gid.toString(8), buffer, offset+116, 8); // gid
+        if ( inode.mtime == undefined ) { inode.mtime = Math.floor((new Date()).getTime()/1000); }
         WriteStringToBinary((inode.mtime).toString(8), buffer, offset+136, 12); // mtime
         //WriteStringToBinary("root", buffer, offset+265, 7);
         //WriteStringToBinary("root", buffer, offset+297, 7); // chksum blank to calculate the checksum
elixx commented 5 years ago

Er, my change reverted after I compiled the source. It looks like here is where the patch needs to be:

diff --git a/js/worker/filesystem/tar.js b/js/worker/filesystem/tar.js
index f7cd924..8482712 100644
--- a/js/worker/filesystem/tar.js
+++ b/js/worker/filesystem/tar.js
@@ -164,6 +164,7 @@ TAR.prototype.Pack = function(path) {
         WriteStringToBinary((inode.mode&0xFFF).toString(8), buffer, offset+100, 8); // mode
         WriteStringToBinary(inode.uid.toString(8), buffer, offset+108, 8); // uid
         WriteStringToBinary(inode.gid.toString(8), buffer, offset+116, 8); // gid
+        if ( inode.mtime == undefined ) { inode.mtime = Math.floor((new Date()).getTime()/1000); }
         WriteStringToBinary((inode.mtime).toString(8), buffer, offset+136, 12); // mtime
         //WriteStringToBinary("root", buffer, offset+265, 7);
         //WriteStringToBinary("root", buffer, offset+297, 7); // chksum blank to calculate the checksum
elixx commented 5 years ago

I wonder if this could be sped up by defining a 'now' variable one level up in scope, and then referencing it for the assignment. So there's only one call out to Math / Date / getTime. I haven't done much JS.

elixx commented 5 years ago

This might not be the best fix. It may be causing buggy file writes / deletions when doing a lot of rapid IOps (like extracting a dense tarball); Some messages about timestamps slightly in the future, and some disappearing files.

s-macke commented 5 years ago

I have probably fixed the problem. Can you check it out, please?

elixx commented 5 years ago

I backed out my change and rolled in your latest commits -- so far, so good. Initial testing is successful.