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

incompatible or wrong C11 Threads return value handling #429

Closed
DenisPolygalov opened this issue Jul 11, 2022 · 4 comments · Fixed by #431 or #476
Closed

incompatible or wrong C11 Threads return value handling #429

DenisPolygalov opened this issue Jul 11, 2022 · 4 comments · Fixed by #431 or #476

Comments

@DenisPolygalov
Copy link

Hi,
the problem is the following code:

if (err) {

You assume that the success code is zero, which is not the case in FreeBSD implementation for example:
https://github.com/freebsd/freebsd-src/blob/3c9ad9398fcdf5f49114fde978b7c837b7ebbc8d/lib/libstdthreads/threads.h#L68

In the standard the value of thrd_success is unspecified:
https://en.cppreference.com/w/c/thread/thrd_errors
So I think return value of all functions related to threads.h must be explicitly compared with members of the enum

Note that the specific line of code in libre source code might be not the only one existed in all 3 projects (re/rem/baresip).
Multiple places where return values get checked might be affected.

Regards,
Denis.

@sreimers
Copy link
Member

sreimers commented Jul 12, 2022

Thanks for report, I'm currently preparing some PRs to fix incorrect return value checks:

libre: #431
librem: baresip/rem#71
retest: baresip/retest#92
baresip: baresip/baresip#1955

@sreimers sreimers changed the title incompartible or wrong C11 Threads calls return values handling incompatible or wrong C11 Threads return value handling Jul 12, 2022
@DenisPolygalov
Copy link
Author

Thank you very much!

@DenisPolygalov
Copy link
Author

I am sorry to tell you this but this is not fixed (does not work de facto). Specifically here:

if (err != thrd_success) {
for example if err is not zero then later
if (err)
mem_deref(m); will be called and non-zero err value returned ruining all the following code. The similarly wrong handling may be present in several other places, such as here:
if (err)
(if the for loop above succeeded). I was able to find only these two.

@sreimers
Copy link
Member

Thanks for pointing this out. I refactored the err handling: #476

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 a pull request may close this issue.

2 participants