Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WIP - moving to Buffer internally from Uint8Array, not passing all tests yet #222

Merged
merged 4 commits into from
Jun 9, 2014

Conversation

humphd
Copy link
Contributor

@humphd humphd commented Jun 4, 2014

This isn't right yet, but it's close. I'm corrupting my data somehow, probably where I've changes the subarray/slice/set Uint8Array calls into copy/slice with Buffers. I could use a hand spotting my errors.

@humphd
Copy link
Contributor Author

humphd commented Jun 4, 2014

We missed each other on irc, but:

15:48 < humph> can you help me?
15:49 < humph> ack: https://github.com/js-platform/filer/pull/222
15:49 < humph> I need some eyes on my logic in the internal methods that chop 
               up bytes and truncate buffers, append, etc.
15:49 < humph> somehow I'm corrupting things
16:11 <@ack> kk, sec
16:21 <@ack> humph: i don't think slice is working like you expect
16:22 <@ack> in the original code, subarray creates a typed array view, but 
             it's backed by the same arraybuffer
16:23 <@ack> are you sure useTypedArrays is true?

Yeah, for sure I'm messing things up here. Can you help me figure out how this kind of stuff should work? I have 3 or 4 places where I do things like this:

-      var newData = new Uint8Array(length);
-      var bufferWindow = buffer.subarray(offset, offset + length);
-      newData.set(bufferWindow);
+      var newData = new Buffer(length);
+      var bufferWindow = buffer.slice(offset, offset + length);
+      bufferWindow.copy(newData);

And I think I'm botching it.

}
var bufferWindow = buffer.subarray(offset, offset + length);
newData.set(bufferWindow, _position);
newData.copy(fileData);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fileData.copy(newData);

@humphd
Copy link
Contributor Author

humphd commented Jun 7, 2014

I've got this all working now. Fixing the zip/unzip stuff sucked because I also had to patch 2 upstream modules:

modeswitch pushed a commit that referenced this pull request Jun 9, 2014
WIP - moving to Buffer internally from Uint8Array, not passing all tests yet
@modeswitch modeswitch merged commit 5cc39b1 into filerjs:develop Jun 9, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants