[PATCH] Use gtk_tree_model_get_value() to get the index and date of a dive

[Dirk Hohndel]

"Lubomir I. Ivanov" <neolit123 at gmail.com> writes:
>>> i think, that is why this is a volatile fix and the real cause might
>>> be a memory issue. i.e. something writing somewhere, where it
>>> shouldn't.
>>> very much possible...and annoyingly hard to debug, especially
>>> considering the fact that GDB somehow crashes the entire OS (Ubuntu)
>>> for me.
>>
>> Yikes. That's encouraging. You might want to submit THAT to Ubuntu for a
>> fix.
>>
>> The thing that really annoyed me is that Valgrind is seeing anything out
>> of the ordinary. Not for the xmlCleanupParser one, nor for this one. If
>> we are stomping all over memory, isn't Valgrind supposed to catch that?
>>
>
> it should in general catch bad things, but i think the runtimes might
> be confusing it a lot. this is definitely the case with windows tools.

That seems to be the case.

>> I pushed all the other fixes. I'm still contemplating this one...
>>
>> There's also Christian's email about cppcheck to look at. Maybe fixing
>> those bugs (the ones that actually ARE bugs) will as a side effect fix
>> this one?
>>
>> I've started on the list and am looking for the actual bugs that could
>> mess up our memory...
>
> there are some memory leaks in parse-xml.c that i've noticed before
> Christian's report.

Leaks are generally harmless. Unless you leak enormous amounts of memory
and run into problems because of that. We leak a few small strings here
and there. Not pretty, but nothing I'm losing sleep over.

> everything else is could be semantics more or less, i'd think.
> "gcc -Wextra -pendantic -std=c99" will spit 500+ lines of such if we
> really want to tackle them.

Well, see my other mail. I think I found a potential smoking gun passing
something that came from malloc to g_free. I haven't looked at glib's
g_free implementation but my gut feeling is that this could cause some
major pain.

> "(style) The scope of the variable..." is a bit out of order
> considering what ANSI C suggested and what some consider clarity.

Yeah, I ignored all of those. We do this in some functions and not in
others. I may end up cleaning them all up just for consistency, but it's
definitely not a correctness issue.

> it seems not to like gcc attributes much.

I notived.

> "sscanf" for the dates is suspicious, but the dates shouldn't be that
> long. there are some buffer lengths to consider in this regard here
> though.

This one I stared at but non of the cases that we see can possibly be
related. Still, I'd appreciate a fix that offers a safer implementation.

> "gmtime_r" not so much for a standalones with linear code. would be
> worth a shot, somehow...

I was worried about compatibility (Windows, Mac), I haven't checked if
they both support this. In general switching to gmtime_r shouldn't cause
any harm.

> the "realloc" thing is a bit bad, but not so much. i mean we should
> fail if it fails - i.e. "if the mapper cannot provide us with a block,
> why even bother".

Yes. We take a rather loose approach with running out of memory. We do
not fail and exit (as might be argued would be a safer approach). Then
again, I cannot see us run out of memory all that easily with real input
data (instead of intentionally trying to crash subsurface).

/D