search

waywardgeek / infnoise

The world's easiest TRNG to get right
Creative Commons Zero v1.0 Universal
723 stars 99 forks source link

Windows driver generates extra 0D characters #8

Closed jironpech closed 9 years ago

jironpech commented 9 years ago

The windows driver unhelpfully appends a hex 0D "character" to every instance of an 0A in the file it writes to make the familiar 0D 0A. This is a typical problem with text files shared between Linux and Windows, but the file should be defined as a binary type so this does not happen. A simple analysis of the file using HxD shows that the 0D bytes are twice as common as everything else. This ruins the entropy, of course.

image

waywardgeek commented 9 years ago

Well, crips! I thought that bug was put to rest ages ago. It probably re-occurred when I pulled a Windows update. The latest way to open the output file is:

outFile = _fsopen(argv[xArg], "w", _SH_DENYWR);

Should it be "wb"?

Thanks for catching this! Bill

On Sat, Aug 15, 2015 at 5:27 AM, jironpech notifications@github.com wrote:

The windows driver unhelpfully appends a hex 0D "character" to every instance of an 0A in the file it writes to make the familiar 0D 0A. This is a typical problem with text files shared between Linux and Windows, but the file should be defined as a binary type so this does not happen. A simple analysis of the file using HxD shows that the 0D bytes are twice as common as everything else. This ruins the entropy, of course.

[image: image] https://cloud.githubusercontent.com/assets/13809344/9288771/33f01746-4359-11e5-9b94-a9fdbace265f.png

— Reply to this email directly or view it on GitHub https://github.com/waywardgeek/infnoise/issues/8.

jironpech commented 9 years ago

OK, that narrows it down a lot - I'll play with that line.

waywardgeek commented 9 years ago

Thanks! I have limited access to Windows machines currently, so if you can verify a fix, that would help a lot.

Bill

On Tue, Aug 18, 2015 at 12:25 PM, jironpech notifications@github.com wrote:

OK, that narrows it down a lot - I'll play with that line.

— Reply to this email directly or view it on GitHub https://github.com/waywardgeek/infnoise/issues/8#issuecomment-132324704.

jironpech commented 9 years ago

Verified - "wb" fixes the output, so thanks for the tip. I can contribute a compiled binary and a few details in the documentation if that helps.

waywardgeek commented 9 years ago

Actually, it would, if you don't mind :)

On Tue, Aug 18, 2015 at 2:20 PM, jironpech notifications@github.com wrote:

Verified - "wb" fixes the output, so thanks for the tip. I can contribute a compiled binary and a few details in the documentation if that helps.

— Reply to this email directly or view it on GitHub https://github.com/waywardgeek/infnoise/issues/8#issuecomment-132355042.

jironpech commented 9 years ago

OK, I created a branch that fixes the 0D error. I also fixed some pathname problems in the VS solution. The latest "daemon" updates broke the windows build. This line in infnoise.h is the culprit: int writePid(pid_t pid, char *fileName);

I don't know if the daemon is intended for the windows version; it sounds like it might be an OS specific tweak. Anyway, the windows and linux versions may not be able to share a header file anymore unless this line can be moved somewhere else or the daemon function can be ported to windows.

As a quick fix I established a new header file for the windows build that omits this line.

I'm not sure how this "pull request" stuff works. What's the best way to review and incorporate these changes?

waywardgeek commented 9 years ago

On Sun, Aug 23, 2015 at 3:24 AM, jironpech notifications@github.com wrote:

OK, I created a branch that fixes the 0D error. I also fixed some pathname problems in the VS solution. The latest "daemon" updates broke the windows build. This line in infnoise.h is the culprit: int writePid(pid_t pid, char *fileName);

I don't know if the daemon is intended for the windows version; it sounds like it might be an OS specific tweak. Anyway, the windows and linux versions may not be able to share a header file anymore unless this line can be moved somewhere else or the daemon function can be ported to windows.

This was a bug. I changed that declaration to make it portable.

As a quick fix I established a new header file for the windows build that omits this line.

I'm not sure how this "pull request" stuff works. What's the best way to review and incorporate these changes?

I merged your changes, and switched back to using infnoise.h. If you get a chance, it would be great if you could test that the merged version compiles in Windows. I manually edited a couple vcproj files where infnoise_win.h occurred, and changed them to infnoise.h, and I am not sure I got it right.

Thanks for the patch!

Bill

jironpech commented 9 years ago

The windows version compiles perfectly - issue closed! Nice teamwork...

waywardgeek commented 9 years ago

Thanks for helping out!

On Tue, Aug 25, 2015 at 3:19 AM, jironpech notifications@github.com wrote:

The windows version compiles perfectly - issue closed! Nice teamwork...

— Reply to this email directly or view it on GitHub https://github.com/waywardgeek/infnoise/issues/8#issuecomment-134547694.