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

fix typo in helper_thread.c and run block_signals(void) function in linux only. #1632

Closed
wants to merge 1 commit into from

Conversation

shailevi23
Copy link
Contributor

fix typo in helper_thread.c and run block_signals(void) function in linux only.

Compiling the code with the typo fix of "#ifdef CONFIG_PTHREAD_SIGMASK" will show that ret is unknown, hence, we are adding the line "int ret;". This fix of typo will fix some other bugs I've encountered.

Also, sigmask works in linux environment, so we will if defined it to linux only.

Signed-off-by: Shai Levy shailevy23@gmail.com.

…inux only.

Compiling the code with the typo fix of "#ifdef CONFIG_PTHREAD_SIGMASK" will show that ret is unknown, hence, we are adding the line "int ret;".
This fix of typo will fix some other bugs I've encountered.

Also, sigmask works in linux environment, so we will if defined it to linux only.

Signed-off-by: Shai Levy shailevy23@gmail.com
@vincentkfu
Copy link
Collaborator

vincentkfu commented Sep 14, 2023

Your new patch will prevent the code from compiling on platforms like BSD and Solaris.

Please follow the suggestion I made in your earlier PR. Evidence suggests that the Windows msys2 build failed because on that platform pthread_sigmask is defined as a noop which triggered the unused variable warning for sigmask.

The configure script detects the existence of pthread_sigmask by trying to compile this program:

#include <stddef.h> /* NULL */
#include <signal.h> /* pthread_sigmask() */
int main(void)
{
  return pthread_sigmask(0, NULL, NULL);
}

Change this program to also trigger the same warning so that compilation also fails. This will make CONFIG_PTHREAD_SIGMASK undefined for the Windows msys2 build and then your original fix will pass all of our testing.

@shailevi23 shailevi23 closed this by deleting the head repository Oct 16, 2023
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