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.

Sign up for GitHub

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

Jump to bottom

Fix hashing context example #91920

Open

dmipeck wants to merge 1 commit into godotengine:master from dmipeck:fix-hashing-context-example

+6 −4

dmipeck commented May 13, 2024

The example function produces invalid hashes for files that aren't a multiple of CHUNK_SIZE bytes in size

Fixes two issues:

replaces use of eof_reached() who's docs say to use file.get_position() < file.get_len() instead when looping
uses remaining number of bytes instead of CHUNK_SIZE if CHUNK_SIZE would be too large

dmipeck requested a review from a team as a code owner

May 13, 2024 20:00

Calinou added bug documentation cherrypick:4.2 labels

Calinou added this to the 4.3 milestone

raulsntos reviewed

View reviewed changes

Member

raulsntos left a comment

Thanks for contributing to the documentation!

CI is complaining about indentation. Make sure the code is indented with 4 spaces, unlike the rest of the XML file which uses tabs.

Also, the commit is linked to a different GitHub account than the one you used to submit the PR. This is fine, but I wanted to make sure it wasn't by mistake. For more information see: Why are my commits linked to the wrong user?

See which "email" you have configured currently: https://github.com/godotengine/godot/pull/91920.patch

doc/classes/HashingContext.xml Outdated

Comment on lines 23 to 24

		while not file.get_position() < file.get_len():
		var remaining = file.get_len() - file.get_position()

Member

raulsntos May 14, 2024

The method is called get_length.

Suggested change

      
            			while not file.get_position() &lt; file.get_len():
          
            				var remaining = file.get_len() - file.get_position()
          
            			while not file.get_position() &lt; file.get_length():
          
            				var remaining = file.get_length() - file.get_position()

doc/classes/HashingContext.xml Outdated

+              			// Update the context after reading each chunk.
+              			while (file.GetPosition() &lt; file.GetLength())
+              			{
+              				int remaining = file.GetLen() - file.GetPosition();

Member

raulsntos May 14, 2024

These methods return ulong so you need to cast the result to int.

Suggested change

      
            				int remaining = file.GetLen() - file.GetPosition();
          
            				int remaining = (int)(file.GetLength() - file.GetPosition());

doc/classes/HashingContext.xml Outdated

+              			while (file.GetPosition() &lt; file.GetLength())
+              			{
+              				int remaining = file.GetLen() - file.GetPosition();
+              				if (remaining &gt; ChunkSize) {

Member

raulsntos May 14, 2024

C# code-style is to add a new line before the curly bracket.

Suggested change

      
            				if (remaining &gt; ChunkSize) {
          
            				if (remaining &gt; ChunkSize)
          
            				{

doc/classes/HashingContext.xml Outdated

+              				if (remaining &gt; ChunkSize) {
+              					remaining = ChunkSize;
+              				}
+              				ctx.Update(file.GetBuffer(ChunkSize));

Member

raulsntos May 14, 2024

To match the GDScript changes:

Suggested change

      
            				ctx.Update(file.GetBuffer(ChunkSize));
          
            				ctx.Update(file.GetBuffer(Mathf.Min(remaining, ChunkSize)));

doc/classes/HashingContext.xml Outdated

+              			# Open the file to hash.
+              			var file = FileAccess.open(path, FileAccess.READ)
+              			# Update the context after reading each chunk.
+              			while not file.get_position() &lt; file.get_len():

Member

raulsntos May 14, 2024

Suggested change

      
            			while not file.get_position() &lt; file.get_len():
          
            			while file.get_position() &lt; file.get_len():

dmipeck force-pushed the fix-hashing-context-example branch 4 times, most recently from e84a6f7 to 570455b Compare

May 14, 2024 07:21


          Fix hashing context example

d6715b4

Example now works for any file size instead of just multiples of CHUNK_SIZE
Example also uses correct method for looping over file data

dmipeck force-pushed the fix-hashing-context-example branch from 570455b to d6715b4 Compare

May 14, 2024 07:24

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment