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

Warnings at compile time #176

Open
rafael2k opened this issue Jan 21, 2023 · 5 comments
Open

Warnings at compile time #176

rafael2k opened this issue Jan 21, 2023 · 5 comments

Comments

@rafael2k
Copy link

Is any of these warnings worrying?

/home/rafael2k/files/rhizomatica/hermes/paq8px/ContextMap.cpp:22:27: warning: writing 1 byte into a region of size 0 [-Wstringop-overflow=]
   22 |     p[1 + ((c >> 5) & 1)] = 1 + ((c >> 4) & 1);
      |                           ^
/home/rafael2k/files/rhizomatica/hermes/paq8px/HashElementForContextMap.hpp:7:11: note: at offset 0 to object ‘bitState’ with size 1 declared here
    7 |   uint8_t bitState;  // state of bit0 (1st slot) / state of bit2 (2nd slot)  /  state of bit5 (in slot 2)
      |           ^
/home/rafael2k/files/rhizomatica/hermes/paq8px/ContextMap.cpp:23:27: warning: writing 1 byte into a region of size 0 [-Wstringop-overflow=]
   23 |     p[3 + ((c >> 4) & 3)] = 1 + ((c >> 3) & 1);
      |                           ^
/home/rafael2k/files/rhizomatica/hermes/paq8px/HashElementForContextMap.hpp:7:11: note: at offset 0 to object ‘bitState’ with size 1 declared here
    7 |   uint8_t bitState;  // state of bit0 (1st slot) / state of bit2 (2nd slot)  /  state of bit5 (in slot 2)
      |           ^
/home/rafael2k/files/rhizomatica/hermes/paq8px/ContextMap.cpp:26:27: warning: writing 1 byte into a region of size 0 [-Wstringop-overflow=]
   26 |     p[1 + ((c >> 2) & 1)] = 1 + ((c >> 1) & 1);
      |                           ^
/home/rafael2k/files/rhizomatica/hermes/paq8px/HashElementForContextMap.hpp:7:11: note: at offset 0 to object ‘bitState’ with size 1 declared here
    7 |   uint8_t bitState;  // state of bit0 (1st slot) / state of bit2 (2nd slot)  /  state of bit5 (in slot 2)
      |           ^
/home/rafael2k/files/rhizomatica/hermes/paq8px/ContextMap.cpp:27:27: warning: writing 1 byte into a region of size 0 [-Wstringop-overflow=]
   27 |     p[3 + ((c >> 1) & 3)] = 1 + (c & 1);
      |                           ^
/home/rafael2k/files/rhizomatica/hermes/paq8px/HashElementForContextMap.hpp:7:11: note: at offset 0 to object ‘bitState’ with size 1 declared here
    7 |   uint8_t bitState;  // state of bit0 (1st slot) / state of bit2 (2nd slot)  /  state of bit5 (in slot 2)
      |           ^
@GotthardtZ
Copy link
Contributor

Thank you for your alertness and reporting these warnings!

These are false positives. The solution in ContextMap is not compiler-friendly, I have to admit.

The compiler is confused by the fact that we are naming each of the 7 bytes in HashElementForContextMap independently (so they look like just 7 independent bytes) and at the same time we are transitioning from one to another with the p pointer (so we are treating the HashElementForContextMap as a 7-byte array). The warning is about this transition.

We could make the warning disappear, if we'd have a 7-byte array in HashElementForContextMap, but then we wouldn't be able to clearly reference these state bytes independently (such as bitState and bitState0, bitState1, etc.)

@L3P3
Copy link

L3P3 commented Jan 21, 2023

You could use constants for the names like byte[MAP_BITSTATE0].

@rafael2k
Copy link
Author

rafael2k commented Jan 21, 2023

Knowing these warnings are false positives is good!
So far, with simple binary data, I got no problem at all. I'm using -3, is that ok? I get good results, sometimes better than higher levels.

@GotthardtZ
Copy link
Contributor

Yes, any compression level is good to use. Lower levels use less memory thus it can't remember patterns encounterd too far in the "past". It makes the modelling more adaptive.
99% of the cases higher levels give better compression ratio, but there are some files with radically changing file content where lower leves work better.

@GotthardtZ
Copy link
Contributor

GotthardtZ commented Jan 22, 2023

@L3P3
Thanks for the suggestion. I may acually do that. (Or use a 7-byte array as another union there. )
Compiler warnings are better to be eliminated.

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

No branches or pull requests

3 participants