Segfault and what's cylinder "use" used for in the planner?

[Rick Walsh]

On 29 Apr 2016 00:38, "Lubomir I. Ivanov" <neolit123 at gmail.com> wrote:
>
> On 28 April 2016 at 11:25, Rick Walsh <rickmwalsh at gmail.com> wrote:
> > Hi,
> >
> > I was just looking at the layout of the planner, and realized that I
> > couldn't find a use for the use field in the available gasses table.
> >
> > We determine whether or not a segment is closed circuit by whether a
> > setpoint has been set in the dive planner points table.  It doesn't look
> > like we actually use the cylinder use value.
> >
> > Can it be removed?
> >
> > Furthermore, the only way I could find to set the value is to type in
the
> > number (0-7) corresponding to the enum.  There's no drop-down of
options,
> > and typing the name of the value doesn't work.  There's also a lovely
> > segfault when I enter the value 8 (or any value greater than 7).  If I
enter
> > a negative number, the text becomes one of the default dive tags.  Yay
> > memory.  Segfault with value 8.
> >
> > (gdb) bt
> > #0  0x00007ffff0e5db3a in strlen () from /lib64/libc.so.6
> > #1  0x00007ffff1a097c0 in QByteArray::QByteArray(char const*, int) ()
from
> > /lib64/libQt5Core.so.5
> > #2  0x00000000006d0105 in gettextFromC::trGettext (this=0xcf5d50,
text=0xd1
> > <error: Cannot access memory at address 0xd1>)
> >     at /home/rick/src/subsurface/core/gettextfromc.cpp:7
> > #3  0x000000000065470a in CylindersModel::data (this=0xec1020,
index=...,
> > role=2) at /home/rick/src/subsurface/qt-models/cylindermodel.cpp:138
>
> ^ that would be a:
>
> cylindermodel.cpp:137:
>
> case USE:
>    ret =
gettextFromC::instance()->trGettext(cylinderuse_text[cyl->cylinder_use]);
>
> where "cyl" is of type "cylinder_t" and "cylinder_use" is of type
> "enum cylinderuse".
> hmm, but "enum cylinderuse" doesn't have 7 elements, it has 3 (or
> NUM_GAS_USE)...
>
> as defined in dive.h:54:
> enum cylinderuse {OC_GAS, DILUENT, OXYGEN, NUM_GAS_USE};
>
> are you sure there are 7 elements and if so where are they defined?

Thanks Lubomir.  Yes, there should only be 3 (+1) elements, with values 0
to 2. But after the cylinderuse elements (0-2), it displayed the
dive_comp_type elements if values 4-7 were entered. 3 was blank.

>
> a patch that can fix the potential buffer overflow of N elements is
attached.

I won't have a chance to test for a few days, but it should be relatively
easy for someone else to verify the bug, and confirm your fix works.

> but it's probably a good thing to also limit the user on the UI side -
> i.e. once a bad value is entered that field should be auto-adjusted to
> a good value.
>
Agreed. I think this should be a drop-down list. Or remove the field from
the table entirely. I couldn't work out if/how it's used at all.

Rick
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.subsurface-divelog.org/pipermail/subsurface/attachments/20160429/7af47b8f/attachment.html>