RFC patch: store dive computer serial / firmware again

Linus Torvalds torvalds at linux-foundation.org
Thu Nov 20 12:48:50 PST 2014


On Thu, Nov 20, 2014 at 12:33 PM, Dirk Hohndel <dirk at hohndel.org> wrote:
> Apparently we did this in the Gtk version of Subsurface, but lost this in
> the Qt transition. Here's a quick patch that should fix this, but I'm not
> quite ready to add this to master until I hear some feedback and a few
> people have tested it.

No, this is simply wrong:

+ if (serial != 0) {
+     snprintf(buffer, sizeof(buffer), "0x%08x", serial);
+     devdata->serial = strdup(buffer);
+ }
+ if (devinfo->firmware != 0) {
+     snprintf(buffer, sizeof(buffer), "0x%08x", devinfo->firmware);
+     devdata->firmware = strdup(buffer);
+ }

Those strings you generate are just random noise, and are actively
misleading and incorrect. They won't actually be the serial numbers at
all. You will just be lying to users, and anybody who uses those
strings will use the wrong strings.

Don't save garbage.

The "serial number" you actually see on the packaging or the device
may be that serial number encoded as decimal. Or hex. Or something
totally different with dashes etc in between. But it will almost
_never_ be that "0x08x" format.

You have two choices:

 - the new DC_FIELD_STRING thing returns a real serial number string,
but only for EON Steel

 - you create some per-dive-computer-manufacturer (possibly per model)
logic in the subsurface libdivecomputer.c, and that then creates the
proper string from the random number you are given. In fact, for
Suunto we already have something like it, because libdivecomputer
*itself* screwed up the format, and different versions return
different things. Older versions returned a "each byte is two digit"
thing (the "native" format - it's not quite BCD, because each byte
would just be 00-99, not 0x00-0x99), while newer versions of
libdivecomputer return the same number but re-encoded as a decimal
number.

I tried to explain to Jef why the whole "it's a number" is complete
garbage, but it seems to have never taken, and when I gave the Suunto
encoding as an example, he just made things worse.

We actually un-re-encode it in
undo_libdivecomputer_suunto_nr_changes() in order to have one single
representatuon. But that code could also basically just format it as a
string.

Add similar cases for other dive computer manufacturers as we figure
out exactly what the encoding actually is.

Saving a serial number that doesn't actually match what you find on
the device, or doesn't actually match what you'd check against recalls
etc, is just wrong and misleading. We're seriously better off with
just the device ID, which doesn't even try to lie and call itself a
serial number. That's true even *if* the device ID changes.

                  Linus


More information about the subsurface mailing list