Closed aefo closed 12 years ago
There are some white space changes / formatting that eclipse did automatically. What settings are you using? (So that they don't keep reverting back & forwards....) I can change mine for this project.
Hey man, thanks for the nice work. I'll look through your changes and merge them in :)
Alright, I've reviewed all changes - I'd like to merge them once we resolve bfb766d - ie. do we really need EncFSFileInputStream.read(byte[], int, int). If we do I'd like to figure out a solution that doesn't overflow the stack on Android before merging. Otherwise it looks great, thanks for all your work!
I've compiled your changes and used it with my Android app, and as predicted I'm running into the issue due to read being overloaded:
E/AndroidRuntime( 359): java.lang.NullPointerException
E/AndroidRuntime( 359): at org.mrpdaemon.sec.encfs.EncFSUtil.byteArrayToLong(EncFSUtil.java:39)
E/AndroidRuntime( 359): at org.mrpdaemon.sec.encfs.EncFSFileInputStream.getBlockIV(EncFSFileInputStream.java:107)
E/AndroidRuntime( 359): at org.mrpdaemon.sec.encfs.EncFSFileInputStream.readBlock(EncFSFileInputStream.java:138)
E/AndroidRuntime( 359): at org.mrpdaemon.sec.encfs.EncFSFileInputStream.read(EncFSFileInputStream.java:192)
E/AndroidRuntime( 359): at java.io.FileInputStream.read(FileInputStream.java:245)
E/AndroidRuntime( 359): at org.mrpdaemon.sec.encfs.EncFSFileInputStream.
The read() in the EncFSFileInputStream() constructor is calling back into EncFSFileInputStream instead of handling the file read itself.
I've tried picking out bfb766d from your changes and keeping the rest, and unfortunately that results in test failure:
Results :
Failed tests: testBoxCryptor_1(org.mrpdaemon.sec.encfs.EncFSVolumeTest): expected:<[test file ]> but was:<[????\??A??]> testBoxCryptor_2(org.mrpdaemon.sec.encfs.EncFSVolumeTest): expected:<[Some contents for file]> but was:<[?E54??3:Vc?O-iC??????]>
Tests run: 8, Failures: 2, Errors: 0, Skipped: 0
I see that you use a BufferedInputStream() - hence why you need read(byte[], int, int) implemented. Since EncFSFileInputStream itself is also a buffered stream, don't you think we could directly use it instead of using it through a BufferedInputStream? If we do so then we don't need read(byte[], int, int). The test passes with the following change:
diff --git a/src/test/java/org/mrpdaemon/sec/encfs/EncFSVolumeTest.java b/src/test/java/org/mrpdaemon/sec/encfs/EncFSVolumeTest.java index bb4e15b..d671da9 100644 --- a/src/test/java/org/mrpdaemon/sec/encfs/EncFSVolumeTest.java +++ b/src/test/java/org/mrpdaemon/sec/encfs/EncFSVolumeTest.java @@ -146,19 +146,14 @@ public class EncFSVolumeTest { ByteArrayOutputStream buf = new ByteArrayOutputStream(); EncFSFileInputStream efis = new EncFSFileInputStream(encFSFile); try {
So I'm inclined to leave out bfb766d from your changes and apply this patch to get the tests working and merge like that. What do you think?
This pull request minus bfb766d plus my test fix from the previous comments works fine with my Android app :)
Merged :)
thanks
for testing...
(plus an additional ignore for eclipse_bin directory)