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 gcc warnings. #49

Merged
merged 2 commits into from
Jan 29, 2017
Merged

Fix gcc warnings. #49

merged 2 commits into from
Jan 29, 2017

Conversation

GandaG
Copy link
Contributor

@GandaG GandaG commented Jan 29, 2017

Fixes the not a prototype warning that thread_hold() gives, fixes the unused argument that comes up after fixing the previous one and fixes the unused argument that thpool_resumes() gives.

@Pithikos
Copy link
Owner

Pithikos commented Jan 29, 2017

Do you think you could provide a few details on how you compile the library and/or how you run it? Compiling the below I don't get any warnings from the compiler and I can run the example.

gcc example.c thpool.c -D THPOOL_DEBUG -pthread -o example

I was getting warnings in case I used the -std=c99 but I fixed that with the latest commit @a3916021c837e4892667e02e1681437ddc362725.

@GandaG
Copy link
Contributor Author

GandaG commented Jan 29, 2017

That example has no warnings enabled, I usually use: -Wall -Wextra -Wstrict-prototypes.

-Wall is absolutely essential and it gives you warnings for the most common and easy to fix mistakes. It doesn't actually enable all warnings.

-Wextra gives warnings about some things that may result in undefined behaviour, like comparing unsigned with signed types.

-Wstrict-prototypes is mostly personal preference but I like to make sure when I look at my forward declarations that I understand what the function is and it doesn't really hurt either way :)

@Pithikos
Copy link
Owner

Pithikos commented Jan 29, 2017

Thanks, it makes sense now.

I think this is ready to go. Just a minor thing. Regarding the (void)thpool_p; in thpool_resume I think it's worth to add a comment making it clear that the idea is to resume threads on a threadpool basis but that has not been implemented yet #13. Else it might be a bit confusing and a bit of danger that someone totally removes the (void)thpool_p; line in the future.

@Pithikos
Copy link
Owner

Thanks for the help!

@Pithikos Pithikos merged commit 0f9b663 into Pithikos:master Jan 29, 2017
@GandaG GandaG deleted the gcc-warnings branch January 29, 2017 18:21
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