On 28/03/17 09:04, Илья Шипицин wrote: > > > 2017-03-28 11:56 GMT+05:00 Gert Doering <g...@greenie.muc.de > <mailto:g...@greenie.muc.de>>: > > Hi, > > On Tue, Mar 28, 2017 at 11:43:11AM +0500, ???????? ?????????????? wrote: > > > (See how dangerous "fixing compiler warnings" is? :-) ) > > > > this particular warning came from cppcheck, it's not a compiler warning > > Same thing. A tool that issues a warning which is not relevant the > way the variables in question are used (because the values stored are > always way below where signed/unsiged gets relevant) - but trying to fix > it, I've misread the surrounding code and the "fixed" code would quite > likely have crashed, or silently corrupted nearby variables. > > > cppcheck says "variable is signed, but format specification is unsigned" > so, I had a look around, it is "byte" variable, which cannot be signed, > so I decided to change it "int" --> "unsigned int" > > later, Gert Doering suggested to switch to uit8_t > > probably, we need to leave variable as is, just change format > specification "%x" --> "%d" ?
Take a moment to re-read what I already quoted from the man page for
sscanf() related to %x:
"Matches an unsigned hexadecimal integer; the next pointer must be a
pointer to unsigned int."
Gert proposed by a mistake to use uint8_t. As integers are most
commonly 32 bits, so uint32_t. But that may be of a different length on
some other platforms. So using 'unsigned int' is most likely the safest
type in this round. Or if wanting to be really portability safe, have a
look at what Simon pointed out, inttypes.h.
Changing from %x to %d will definitely cause havoc. This function
[parse_hash_fingerprint()] is related to parsing fingerprint hashes into
an integer value. Certificate fingerprint hashes are most commonly
longer strings of hexadecimals. So such a change will actually make
sscanf() fail horrendously when the characters a-f appears. %d does not
handle hexadecimal numbers.
Again, as Gert have already said twice ... it is important to get an
understanding of what the code does before trying to change it. Code
analysers doesn't necessarily provide the only sane truth, as most of
them is incapable of understanding the broader context. I do not say
such analysers are useless or pointless, merely that each warning must
first be thoroughly reviewed and considered before just jumping into
changing things.
--
kind regards,
David Sommerseth
OpenVPN Technologies, Inc
signature.asc
Description: OpenPGP digital signature
------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot
_______________________________________________ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
