Fix warnings #21
Open
Fix warnings #21
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
against my const-obsessive and error-code obsessive
instincts. None of these changes were the result of
behavioral program bugs, but are fixes to *minorly*
incorrect code, so that it compiles cleanly.
I've fixed 2 types of warnings that I noticed when I went
and built i7z from source:
1. Format string warnings like:
```
i7z.c:187:34: warning: format ‘%d’ expects argument of type ‘int’, but argument 3 has type ‘__time_t {aka long int}’ [-Wformat=]
fprintf(fp_log_file_freq,"%d.%.9d\n",value->tv_sec,value->tv_nsec); //n
^
i7z.c:187:34: warning: format ‘%d’ expects argument of type ‘int’, but argument 4 has type ‘__syscall_slong_t {aka long int}’ [-Wformat=]
```
...where the string specified a basic signed integer with `%d`,
and instead the underlying type was `long int`. I fixed this by
simply changing the format string to `%ld`.
2. Ignored return values, like:
```
helper_functions.c: In function ‘cpufreq_info’:
helper_functions.c:551:5: warning: ignoring return value of ‘system’, declared with attribute warn_unused_result [-Wunused-result]
system
^
helper_functions.c:559:5: warning: ignoring return value of ‘fgets’, declared with attribute warn_unused_result [-Wunused-result]
fgets (tmp_str, 30, tmp_file);
```
...where the function returns an error code that library authors
thought was important to not ignore. For both the `system`
function calls and the `fgets` function calls, I simply log
failure to `stderr` - nothing to fancy or aggressive (like `abort()`).
For the `system` function calls, I tried not to be *too* generic,
and only paid attention to error codes from calls that i7z
*actually* executes, and mention them in a comment.
For `fgets`, I've zero-initialized the output buffer, so that an
`fgets` failure will not result in `atof` reading junk.
I made three other types of small changes:
1. Zero-initializing a few variables that are passed to library
functions. I did this instead of checking return values (or both)
to reduce code churn. This wasn't the result of a warning, but I
did it anyways.
2. Changing a few of the cstring handling functions to their
semi-bounds checked versions. Since we don't have C11 Annex K,
*(thanks Ulrich Drepper!)* I had to use `snprintf` instead of
`sprintf_s` (or `snprintf_s`) - which is practically equivalent.
This is the only behavioral change, and even then it should not
actually change behavior unleess i7z was overrunning a buffer
somewhere as-is. Look carefully at this part please?
I also *considered* changing array arguments with compile-time sizes
as `[static 10]` (where 10 is the array's size) instead of an
unsized pointer. In the cases where I considered this, it's more
expressive, and the compiler would've generated an error if too
small a buffer was passed. This would be simpler (and more
reliable without annotations) than a separate size argument.
Lastly, I put some `FIXME:`s in where the compiler generated
warnings with `-Wall -Wpedantic`. I havent modified the makefile
in this patch, I just did it to test my changes.
ariccio
commented
Dec 21, 2017
| @@ -114,7 +114,7 @@ static inline void cpuid (unsigned int info, unsigned int *eax, unsigned int *eb | |||
| if (edx) *edx = _edx; | |||
| } | |||
|
|
|||
| static inline void get_vendor (char *vendor_string) | |||
| static inline void get_vendor (char vendor_string[static 13]) | |||
There was a problem hiding this comment.
Whoops. looks like I said that I didn't do this in my commit message, but I still did. Behavior is nonetheless correct.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I've said most everything in my commit message for de11b91
...but in short, when I went to build i7z from source to use it, gcc spat out several warnings, so I fixed them. I tried to keep the changes minimal to reduce code churn. I've also tweaked the string formatting where they weren't generating warnings, but a minimal change would be technically safer. I'm not sure how you feel about PRs, but hopefully this is fine.