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.
Issue #, if available: N/A
Description of changes:
This PR addresses the issues identified by GitHub's Code Scanning alerts, as well as a few others that are just a drop in the ocean of warnings ion-c currently spits out. I'm hoping to spend some time over the next few weeks cleaning up more, so hopefully this will be just the first PR in a series.
Pre-PR:
Post-PR:
Some Explainations
Int & FILE * conversions
I added a couple of macros to
ion_stream.c
calledFD_TO_FILEP
andFILEP_TO_FD
. The ION_STREAM struct contains a field_fp
that is of typeFILE *
, but is also used to store an integer file descriptor. When converting a pointer to an integer, C99 6.3.2.3.6 comes into play:On x86_64 systems, a pointer is 64-bit, and an int is 32-bit, so the operation generates a warning. When converting from the integer to the pointer, the compiler also emits a warning, my guess is because of the same rule. The macros add a
size_t
which is not a pointer, and is a 64-bit value so it allows conversion to/from an int without warning, and to/from a pointer since the sizes match.Redundant looking
char *
'sThere are a few added type casts to strings that look redundant, such as
(char *)"str_col1"
. In C a string literal is considered achar *
, at least up to C99. In C++11 (I think) C++ diverged from this idea and (rightfully) changed the string literal (which is usually kept read-only memory) to aconst char *
, a pointer to constant characters. Historically, most functions that take a string as an argument, consider the string to beconst char *
, but sometimes that is not the case, like withion_string_assign_cstr
, which takes achar *
argument, even though no modification to the characters occur. So these type casts were used to stop the C++ compiler from complaining about dropping const, which rightfully could indicate a bug.Integer Overflow in Computation
In
argtable3.c
, there is a calculation that involves multiple integers. The end result of the computation is used as asize_t
argument toxmalloc
. Implicit conversion from an int to size_t occurs in order to use the result as the argument, but that conversion occurs after the computation is complete. During the computation, it is possible that the integers involved, will overflow the integer storage, before the implicit conversion occurs. So I've added type casts to the values themselves so that each operation in the computation is considered a size_t. (This is all from the compiler's perspective, I don't think, in this situation, there's any chance that the argument parser could calculate memory requirements that exceed 32bit values)By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.