Several float to int potential rounding errors/inconsistencies

[Jérémie Guichard]

Hey guys,

Thanks for the help on this topic!

I have now updated my pull request with the different proposed changes.
The first commit changes all rint to lrint. Based on Lubomir
recommendation, I went through the different places to check (and use)
lrintf where necessary. Doing so I found two places that looked strange to
me since (to my understanding) it was using rint on the result of an
integer division, this seemed curious! This is why I changed the code to
use double/float division. Please have a closer look there:

In dive.h the gas_mnd function:
rint(mbar_to_depth(maxambient, dive) / roundto) * roundto;

In parse-xml.c the divinglog_profile function:
rint(atoi(cns) / 10)

Second commit, fixes all warnings raised by "-Wfloat-conversion", I made
the change from round to lrint in uemis code too.

Unfortunately I could not enable the flag in that commit since travis seems
to be using gcc (Ubuntu 4.8.4-2ubuntu1~14.04.3) 4.8.4 that do not include
this option... Nevertheless it may be a good idea to make use of the option
so should we:
1. Upgrade travis build to use more recent version of gcc and add the flag
unconditionally?
2. Add the flag under a cmake gcc version condition so that people using
more recent version of gcc in their dev env be warned about the mistakes?
3. Since 1 could be annoying to people still using older gcc in their dev
env, we could upgrade travis but still put the flag behind a condition like
proposed in 2?

With this change, TestParse will now pass on Windows too, we now only have
TestPreference failing! Getting there :)

Best regards,

Jérémie

2017-03-08 9:21 GMT+07:00 Jérémie Guichard <djeBrest at gmail.com>:

> I'm looking into that right now :)
>
> 2017-03-08 7:18 GMT+07:00 Dirk Hohndel <dirk at hohndel.org>:
>
>> On Tue, Mar 07, 2017 at 09:43:06AM -0800, Linus Torvalds wrote:
>> > On Tue, Mar 7, 2017 at 9:20 AM, Lubomir I. Ivanov <neolit123 at gmail.com>
>> wrote:
>> > >
>> > > lrint() seems to build for me with mingw 4.9.2.
>> >
>> > .. in fact, I notice that we had a couple of places in the GPS
>> > coordinates code that already used 'lrint()', so it must work.
>> >
>> > The uemis code has a few places that use 'round()' instead of rint() -
>> > those should probably be converted to 'lrint()' as well.
>>
>> Would you be willing to run the script, make the other changes you
>> suggest, and send a patch / pull request?
>>
>> I'm really swamped this week.
>>
>> /D
>>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.subsurface-divelog.org/pipermail/subsurface/attachments/20170308/085a2a55/attachment-0001.html>