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

types: add RE_ prefix to ARRAY_SIZE() #658

Merged
merged 3 commits into from
Jan 25, 2023
Merged

types: add RE_ prefix to ARRAY_SIZE() #658

merged 3 commits into from
Jan 25, 2023

Conversation

alfredh
Copy link
Contributor

@alfredh alfredh commented Jan 24, 2023

  1. avoid potential naming conflicts
  2. add prefix to make the code more readable

@alfredh
Copy link
Contributor Author

alfredh commented Jan 24, 2023

the CI failed with a new error:

test: test_sipsess_100rel_answer_not_allowed: test failed (Invalid argument [22])

this is not related to this change. Is it related to the PRACK commit yesterday ?

@cspiel1 could you please take a look ?

@cspiel1
Copy link
Collaborator

cspiel1 commented Jan 25, 2023

Could no reproduce this so far.

Where did you see the line?

test: test_sipsess_100rel_answer_not_allowed: test failed (Invalid argument [22])

On my dev host in home office with Ubuntu 22.04 and openssl 3.0.2. I still get this with valgrind

test 2 -- test_aes_gcm
==1302671== Conditional jump or move depends on uninitialised value(s)
==1302671==    at 0x4FB52A4: ??? (in /usr/lib/x86_64-linux-gnu/libcrypto.so.3)
==1302671==    by 0x4FB5581: ??? (in /usr/lib/x86_64-linux-gnu/libcrypto.so.3)
==1302671==    by 0x4EB60F4: EVP_DecryptFinal_ex (in /usr/lib/x86_64-linux-gnu/libcrypto.so.3)
==1302671==    by 0x4908C0D: aes_authenticate (aes.c:240)
==1302671==    by 0x11F4A5: test_aes_gcm (aes.c:341)
==1302671==    by 0x16661B: test_unit (test.c:510)
==1302671==    by 0x166EBC: test_reg (test.c:728)
==1302671==    by 0x1401F3: main (main.c:233)
==1302671== 

On my dev host at Commend office (also Ubuntu 22.04 and openssl 3.0.2) didn't had this. This is a different problem but hinders me to investigate the 100rel test. We will open a PR that adds a valgrind suppression file.

edit: As long as the conditional jump warning doesn't occur on github we don't need a suppression file in the repository.

@maximilianfridrich
Copy link
Contributor

maximilianfridrich commented Jan 25, 2023

This is definitely due to my changes in baresip/retest#167. It seems to be a timing issue in retest, I will investigate it. Sorry for causing the tests to be unstable.

@maximilianfridrich
Copy link
Contributor

This re PR could fix it: baresip/retest#177. Also, my comment on the retest issue baresip/retest#176 explains why I think this is happening.

@sreimers sreimers merged commit e281bc6 into main Jan 25, 2023
@sreimers sreimers deleted the arraysize_prefix branch January 25, 2023 11:25
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.

4 participants