Hi,
I understand the intent here and its nice to get rid of compiler warnings,
when
it makes sense. But be careful while silencing by just a cast.
On Sun, Nov 5, 2017 at 5:06 PM, Simon Rozman <si...@rozman.si> wrote:
>
> Hi,
>
> Let me explain all proposed modifications case-by-case below...
>
> > > diff --git a/src/openvpn/block_dns.c b/src/openvpn/block_dns.c index
> > > d43cbcf..f88ba2c 100644
> > > --- a/src/openvpn/block_dns.c
> > > +++ b/src/openvpn/block_dns.c
> > > @@ -370,7 +370,7 @@ get_interface_metric(const NET_IFINDEX index,
> > const ADDRESS_FAMILY family)
> > > }
> > > return ipiface.Metric;
> > > }
> > > - return -err;
> > > + return -(int)err;
> > > }
> >
> > This, I can somewhat agree to, as "err" is an unsigned type so there
might be
> > a hidden integer overflow if it happens to be "large". Which it won't
be, but
> > still.
>
> Adding the `(int)` resolves the warning C4146: unary minus operator
> applied to unsigned type, result still unsigned.
> https://msdn.microsoft.com/en-us/library/4kh09110.aspx
>
> It doesn't change much, but it shut-ups one compiler warning.
The original code could be argued to be logically wrong (though
perfectly legal C) if err becomes larger than a signed int. This change
does not fix that. So what's the point?
As an example, consider this
err = 2147483649 <(214)%20748-3649> (2 more than INT_MAX)
Original will return 2147483647 <(214)%20748-3647> (not a -ve int)
The new code will return the same 2147483647 <(214)%20748-3647>
That is, the return value is +ve int which the caller will wrongly
interpret as valid result.
This is hypothetical as Windows system err codes do not get that large.
But then the original is as good as the replacement except for a
C++-trained compiler being silenced.
While most of the MSVC compiler warnings are false-alarms, one out of many
> does represent a potential threat. But you don't know which one that is.
> Therefore it is best to address them all. :)
>
To repeat, cast does not eliminate a potential for error, it just just
hides it.
Selva
------------------------------------------------------------------------------
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
