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

Direct Padding #306

Merged
merged 1 commit into from
Aug 8, 2020
Merged

Direct Padding #306

merged 1 commit into from
Aug 8, 2020

Conversation

Snaggly
Copy link
Contributor

@Snaggly Snaggly commented Aug 6, 2020

Jumps straight to desired address without looping through the file while loading or saving. By default, jumping to a higher address on a new file will zero the previous bytes, so there is no need to constantly write zeros. This may increase performance...

Jump straight to desired address without looping through the file.
@colinator27
Copy link
Member

I doubt this will increase performance, but it's nice to have nonetheless I suppose. Though, is that behavior of filling in zero bytes documented anywhere? I just want to make sure it doesn't break on obscure implementations or something.

@Snaggly
Copy link
Contributor Author

Snaggly commented Aug 6, 2020

https://docs.microsoft.com/en-us/dotnet/api/system.io.filestream.position?view=netcore-3.1#System_IO_FileStream_Position
Under Remarks: "In Microsoft Windows NT and newer, any data added to the end of the file is set to zero."

@colinator27
Copy link
Member

Hmmm I'm a bit worried, then, since that might cause issues with things like Mono

@Snaggly
Copy link
Contributor Author

Snaggly commented Aug 6, 2020

I'm using Wine+Mono here, it works as it should. Though that's all I can say, as I don't know exactly how they implemented the handling here.

@colinator27
Copy link
Member

So I'll merge it now, and if anybody reports any issues in the future, we could always revert it, but hopefully that won't be the case.

@colinator27 colinator27 merged commit 0deafb1 into UnderminersTeam:master Aug 8, 2020
@colinator27
Copy link
Member

Okay so, apparently this is currently causing issues for some people, and now that I think about it, skipping those bytes while reading is a bad idea in the first place (as it doesn't actually verify the padding), so I'm going to revert this for now.

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.

2 participants