kicad-developers team mailing list archive
-
kicad-developers team
-
Mailing list archive
-
Message #26486
Re: [PATCH] Remove check for undefined behaviour
Just in case NULL is indeed deliberately cast to a ref (as in the case of
OpenCascade), how about replacing this test with something like:
void foo(int &i)
{
unsigned long long z = (unsigned long long) &i;
if( z < 2 )
std::cout << "blah";
}
Of course if the compiler is *too clever* it will realize that "2" is
still an invalid
pointer on 32 and 64-bit addressing schemes and still optimize this out, but
my gcc 6.2.0 isn't that clever. The only *other* problem is that
unsigned long long
is to be at least 64b but I have no idea if there is anything in the language
specification to guarantee it is at least as large as a pointer type.
- Cirilo
On Thu, Sep 29, 2016 at 1:57 AM, Wayne Stambaugh <stambaughw@xxxxxxxxx> wrote:
> I didn't test what the compiler generated but I suspected that was the
> case from warning message. If that's the case then I will apply the
> patch but it does mask the underlying issue which could fail spectacularly.
>
> On 9/28/2016 11:43 AM, Chris Pavlina wrote:
>> Well, as for removing them being a good or bad idea - the compiler is
>> probably doing that anyway. I just tested on GCC, at -O1 or greater it
>> just assumes all references are non-NULL and replaces any "&ref != 0"
>> with a hard-coded boolean 'true'. So... there's a pretty good chance we
>> haven't had those checks for quite some time.
>>
>> On Wed, Sep 28, 2016 at 09:24:40AM -0400, Wayne Stambaugh wrote:
>>> I normally don't like to rant but OMFG! I just looked at the code path
>>> that lead to this wonderful piece of code and I'm now regretting that.
>>> You just cannot unsee that. See if you can follow the logic (assuming
>>> logic was actually used to write this code) backwards from
>>> Triangle::NeighborAcross(). I'm not sure removing the invalid null
>>> reference checks is a good idea even though I've seen the compiler
>>> complain about it and agree with it. In what universe do you blindly
>>> convert a potentially NULL pointer into a C++ reference, pass it to the
>>> caller, and go merrily on you way? This really needs to be rewritten
>>> using pointers with the appropriate NULL pointer checks along the way.
>>> It's hard to imagine this ever worked properly.
>>>
>>> On 9/27/2016 1:33 PM, Simon Richter wrote:
>>>>
>>>> Triangle::NeighborAcross returns a reference, which must refer to a valid
>>>> object whose address cannot be 0. Thus, this test is nonsensical.
>>>> ---
>>>> polygon/poly2tri/sweep/sweep.cc | 16 ----------------
>>>> 1 file changed, 16 deletions(-)
>>>>
>>>>
>>>>
>>>> _______________________________________________
>>>> Mailing list: https://launchpad.net/~kicad-developers
>>>> Post to : kicad-developers@xxxxxxxxxxxxxxxxxxx
>>>> Unsubscribe : https://launchpad.net/~kicad-developers
>>>> More help : https://help.launchpad.net/ListHelp
>>>>
>>>
>>> _______________________________________________
>>> Mailing list: https://launchpad.net/~kicad-developers
>>> Post to : kicad-developers@xxxxxxxxxxxxxxxxxxx
>>> Unsubscribe : https://launchpad.net/~kicad-developers
>>> More help : https://help.launchpad.net/ListHelp
>
> _______________________________________________
> Mailing list: https://launchpad.net/~kicad-developers
> Post to : kicad-developers@xxxxxxxxxxxxxxxxxxx
> Unsubscribe : https://launchpad.net/~kicad-developers
> More help : https://help.launchpad.net/ListHelp
Follow ups
References