Re: o3tl::make_unsigned

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Friday 31 of January 2020, Kaganski Mike wrote:
> On 2020-01-30 17:25, Luboš Luňák wrote:
> >   Not worth it. That'd be like doing error checking for every memory
> > allocation - we also bother only with those few cases where it
> > realistically can go wrong.
>
> I disagree with this approach. Not checking memory allocation result is
> a strategy with specific and easily controlled results of failed
> expectation. I am sure that it will segfault, not proceeding with wrong
> operation - and that's enough for me.

 That's incorrect, actually. On Linux running out of memory will in fact 
usually not fail the memory allocation but instead you'll run out of RAM and 
the system will be brought to a halt by thrashing until the OOM-killer takes 
its sweet sweet time and maybe does something after an hour.

 But ok, maybe a better example is something like "average = ( a + b ) / 2" 
and cases like that, which I assume we have plenty of, and we usually don't 
bother checking, because "it'll never happen". We check in special places 
where it possibly could go wrong, but generally we don't. We can't check 
everything and there's a line behind which it's not worth bothering.

> IMO the "let's change make_unsigned with make_signed" only makes sense
> if it is *correct* solution, even if it implies overhead.
>
> My take on this is https://gerrit.libreoffice.org/c/core/+/87762.

 This may be theoretically correct, but I consider it to be a needless 
overkill. Just like we do not bother to verify every single integer operation 
for overflow because it's not worth it, the same way I think we shouldn't 
bother here, besides a possible assert(), because it's not worth it (and I 
still have to see a single plausible case where this could realistically go 
wrong).

-- 
 Luboš Luňák
 l.lunak@xxxxxxxxxxxxx
_______________________________________________
LibreOffice mailing list
LibreOffice@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/libreoffice




[Index of Archives]     [LARTC]     [Bugtraq]     [Yosemite Forum]     [Photo]

  Powered by Linux