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

handle EOF in File::getVLI() #27

Merged
merged 1 commit into from
Dec 22, 2024
Merged

handle EOF in File::getVLI() #27

merged 1 commit into from
Dec 22, 2024

Conversation

micsthepick
Copy link
Contributor

convert EOF to 0 to prevent infinite loop.

@micsthepick
Copy link
Contributor Author

see also: hxim/paq8px#183

@kaitz
Copy link
Owner

kaitz commented Dec 15, 2024

I have not yet tested this.
Also one text string is changed. Char encoding type problem.

@micsthepick
Copy link
Contributor Author

Hmm, maybe it would be better to use escape sequences. The editor I used was either VSCode or Nano.

@micsthepick
Copy link
Contributor Author

micsthepick commented Dec 15, 2024

can't see that diff locally:

git diff HEAD~
diff --git a/file/File.cpp b/file/File.cpp
index 820d1da..b6f2e99 100644
--- a/file/File.cpp
+++ b/file/File.cpp
@@ -20,7 +20,7 @@ uint64_t File::getVLI() {
   int k = 0;
   uint8_t b = 0;
   do {
-    b = getchar();
+    b = max(0, getchar());
     i |= static_cast<uint64_t>(b & 0x7F) << k;
     k += 7;
   } while((b >> 7) > 0 );
mike@JOIN:~/paq8px2$ git branch
  master
  oldmain
* patch_1
  patch_2

@micsthepick
Copy link
Contributor Author

micsthepick commented Dec 15, 2024

sha256sum file/File.cpp
28d1c551caf220fa7ef93c5d8e1a59514e16c002e67fa707612e13fa08a89919  file/File.cpp
...
git checkout HEAD~
Note: switching to 'HEAD~'.
...

HEAD is now at 37fb9c8 Merge pull request #180 from GotthardtZ/master
mike@JOIN:~/paq8px2$ sha256sum file/File.cpp
39d1b4cb18cd81e6581f07649aab441a92f3beda5fa4ebc11300871bffad20f3  file/File.cpp

Copy link
Contributor Author

Choose a reason for hiding this comment

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

the second diff here looks to be a Github specific bug

@micsthepick
Copy link
Contributor Author

see the discussion I raised at https://github.com/orgs/community/discussions/147176

@kaitz kaitz merged commit 5c0ae7b into kaitz:master Dec 22, 2024
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