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 format warnings of type 'pid_t' #53

Merged
merged 1 commit into from Feb 26, 2019

Conversation

Low-power
Copy link
Contributor

Solaris has defined type pid_t as long int on 32-bit platforms, so using %d in a format string for pid_t will cause a compiler warning about the type mismatch.

@codecov-io
Copy link

codecov-io commented Feb 26, 2019

Codecov Report

Merging #53 into master will increase coverage by 0.06%.
The diff coverage is 40%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #53      +/-   ##
==========================================
+ Coverage   50.26%   50.32%   +0.06%     
==========================================
  Files          33       33              
  Lines       13432    13432              
==========================================
+ Hits         6751     6760       +9     
+ Misses       6681     6672       -9
Impacted Files Coverage Δ
lib/facil/redis/redis_engine.c 0% <0%> (ø) ⬆️
lib/facil/fio.c 58.82% <46.15%> (+0.19%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 25f1229...cc30189. Read the comment docs.

@boazsegev
Copy link
Owner

Interesting! Thanks for the PR 👏🏻☺️🙏🏻

Question... wouldn't long int have a higher range? how high do Solaris process number go?

Maybe we should change the printout upwards (use %zd and cast all pid_t to ssize_t)...?

@Low-power
Copy link
Contributor Author

Nope, long int has same size with int on Solaris i386 (32-bit).

@boazsegev boazsegev merged commit ed59989 into boazsegev:master Feb 26, 2019
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.

None yet

3 participants