[PATCH] Properly initialize device_data_t when downloading

Tomaz Canabrava tcanabrava at kde.org
Sat Jun 15 21:07:06 PDT 2013

On Sat, Jun 15, 2013 at 9:56 PM, Linus Torvalds
<torvalds at linux-foundation.org> wrote:
> From: Linus Torvalds <torvalds at linux-foundation.org>
> Date: Sat, 15 Jun 2013 14:48:31 -1000
> Subject: [PATCH] Properly initialize device_data_t when downloading
> The old gtk branch started out with device_data_t explicitly cleared,
> but the Qt version never did that.  And we actually depend on the
> deviceid in particular being initialized to zero (and then we fill in
> the details in the divecomputer download callbacks)
> Not properly initializing it meant that we ended up with random
> deviceid's that got added to the divecomputer device lists, and then
> saved to the XML file without actually matching the data in the dive
> computers in the actual *dives*.
> Signed-off-by: Linus Torvalds <torvalds at linux-foundation.org>
> ---
> This was way more subtle than I'd like. The new device_data_t
> initialization is messy. We should zero it all.
> The gtk branch initializes the whole structure when it declares it:
>         device_data_t devicedata = {
>                 .devname = NULL,
>         };
> but the Qt branch uses this implicit "data" field that is never
> initialized, because it's part of "this". I think that's bad form, never
> mind whether it might be "normal" for C++ people to do things like that.
> This is the minimal "at least initialize the two deviceid/diveid fields".
> I think we'd be better off clearing it all with a proper initializer, but
> I'm not going to fight the C++ model.


All the data ( and most of the code ) should be in the C form, if I messed
with anything - it's my fault since I did the code for the download from dive
computer, It can safely be brougth back to C code. I most likely used the C++
form because I'm very used to this - so I didn't think that could bebad ( or
worse, introduce regressions. )

Also, you said that the Cylinder / Weigth editions are worse than in GTK
version, can you describe briefely why ? - remind you that I never dived or
used the program before - I'm helping to the Qt Transition becaus I think is
fun to hack, but never really using the software means that I sometimes can
make a change that I think is for the best, and actually make things worse, I
can deal with that, It's easy to revert something and do in another way.

One last thing, when I started the Qt branch I did a very big skeleton of the
UI and started to fill the gaps but some stuff has a different
behavior in Qt and
GTK, so that's why new code is being added, anything that you spot that can
still be the old C code, just point me and I'll do my best to make it.

Tomaz ( recovering from the police brutality )
-- side note, search for são paulo police brutality, quite fun pictures and
you can spot me in one or two.

>  qt-ui/downloadfromdivecomputer.cpp | 1 +
>  1 file changed, 1 insertion(+)
> diff --git a/qt-ui/downloadfromdivecomputer.cpp b/qt-ui/downloadfromdivecomputer.cpp
> index b0bdea7399d5..37ca2775bca0 100644
> --- a/qt-ui/downloadfromdivecomputer.cpp
> +++ b/qt-ui/downloadfromdivecomputer.cpp
> @@ -157,6 +157,7 @@ void DownloadFromDCWidget::on_ok_clicked()
>         data.product = strdup(ui->product->currentText().toUtf8().data());
>         data.descriptor = descriptorLookup[ui->vendor->currentText() + ui->product->currentText()];
>         data.force_download = ui->forceDownload->isChecked();
> +       data.deviceid = data.diveid = 0;
>         set_default_dive_computer(data.vendor, data.product);
>         set_default_dive_computer_device(data.devname);
> --
> 1.8.3
> _______________________________________________
> subsurface mailing list
> subsurface at hohndel.org
> http://lists.hohndel.org/cgi-bin/mailman/listinfo/subsurface

More information about the subsurface mailing list