[Subsurface-divelog/subsurface] Application crashes immediately when typing a comma into coordinates (#158)

[Dirk Hohndel]

On Sat, Jan 14, 2017 at 05:26:23PM +0100, Robert Helling wrote:
> 
> > On 14 Jan 2017, at 15:51, hhmeyer <notifications at github.com> wrote:
> > 
> > I use Subsurface 4.5.6 on a Mac with Mac OS X 10.11.6.
> > When I edit the data for a location and enter the coordinates manually, typing a comma immediately crashes the program. I have set the format of coordinates to decimal.
> 
> I see this as well for the beta but not in my self built binary, which makes this a bit hard to debug (the beta does not have debug symbols). Could it be that it is about the Qt version (I am still in 5.5)?

4.5.6 was built with 5.6 IIRC (easy to check if you open the DMG). The
current Beta is built with 5.7.1

> Here is what I get: On the console there is
> 
> ASSERT: "uint(i) < uint(size())" in file /Users/hohndel/Qt/5.7/clang_64/lib/QtCore.framework/Headers/qstring.h, line 881
> Abort trap: 6

Yep, I see the same thing.

> 5   org.qt-project.QtCore         	0x0000000104e0b40c qt_assert(char const*, char const*, int) + 76
> 6   org.subsurface-divelog        	0x00000001015f1695 0x101390000 + 2496149
> 7   org.subsurface-divelog        	0x00000001015e11dd 0x101390000 + 2429405
> 8   org.subsurface-divelog        	0x00000001015e0510 parseGpsText(QString const&, double*, double*) + 1904
> 9   org.subsurface-divelog        	0x00000001014bee8c LocationInformationWidget::on_diveSiteCoordinates_textChanged(QString const&) + 204

So we call into parseGpsText but then don't see what it calls which causes
the assert.

Running this here under the debugger it's actually painfully obvious (and
I wonder a bit why this doesn't show up with the version that you built
from source and why this doesn't show up on other OSs):

		} else if ((numberDefined || minutesDefined || secondsDefined) &&
			   (txt[pos] == ',' || txt[pos] == ';')) {
			// next coordinate coming up
			// eat the ',' and any subsequent white space
			while (txt[++pos].isSpace())
				/* nothing */ ;
			break;

Look at that code and imagine typeing a ',' at the very end of a string.
The assert that is shown (that checks if the position we access is less
than the length of the string) will obviously trigger.

I'll fix that bug.

/D