On Mon, Apr 23, 2018 at 10:07 AM, Linus Torvalds
<[email protected]> wrote:
>
> What we *could* do is to do it knowingly wrong, the way dctool does
> it: don't save the fingerprint data with the real dive data at all,
> just have a random "last download" cache indexed by device family and
> serial number.
>
> That's how the feature is designed to be used, and it would avoid the
> nasty "pollute our logs with meaningless and badly defined crazy data
> that will never be useful in the future".

So here's a patch for people to play with if they want to.

This is literally how the fingerprint thing is meant to be used, and
it works, but it's a bit dangerous.

It's dangerous because the rule is "once we've downloaded the dives on
one machine, we don't want to download them again".

You can see the breakage by:

 (a) applying this patch

 (b) downloading some dives into a new empty xml file, or as a
different user (so that you actually download things)

 (c) *NOT* saving the end result, closing subsurface

 (d) opening subsurface again with the same empt xml file, and trying
to download

At that point, the fingerprint code says "I've already downloaded all
the dives on this machine, so you have nothing new to download".

Which is exactly how the feature is designed, so hey, it's doing what
it meant to do. You don't want to re-download the same dives over and
over again, do you?

And if you *do* want to re-download them, you'd better just check the
"Force download all dives" checkbox.

Note that this patch saves the fingerprint in the downloader. It would
probably be better to at *least* wait until the user has pressed "ok"
with the most recent dive checked. That still doesn't mean "saved",
but it's a lot closer.

This has been very lightly tested, and did the right thing with my Perdix AI.

                      Linus
 core/libdivecomputer.c | 91 ++++++++++++++++++++++++++++++++++++++++++++++++++
 core/libdivecomputer.h |  2 ++
 2 files changed, 93 insertions(+)

diff --git a/core/libdivecomputer.c b/core/libdivecomputer.c
index c547a85b2..1f7eec2f5 100644
--- a/core/libdivecomputer.c
+++ b/core/libdivecomputer.c
@@ -8,6 +8,9 @@
 #include <unistd.h>
 #include <inttypes.h>
 #include <string.h>
+#include <sys/types.h>
+#include <sys/stat.h>
+#include <fcntl.h>
 #include "gettext.h"
 #include "dive.h"
 #include "device.h"
@@ -21,6 +24,9 @@
 
 #include "libdivecomputer.h"
 #include "core/version.h"
+#include "core/qthelper.h"
+#include "core/membuffer.h"
+#include "core/file.h"
 
 //
 // If we have an old libdivecomputer, it doesn't
@@ -778,6 +784,14 @@ static int dive_cb(const unsigned char *data, unsigned int size,
 		goto error_exit;
 	}
 
+	if (!devdata->fingerprint) {
+		devdata->fingerprint = calloc(fsize, 1);
+		if (devdata->fingerprint) {
+			devdata->fsize = fsize;
+			memcpy(devdata->fingerprint, fingerprint, fsize);
+		}
+	}
+
 	import_dive_number++;
 	dive = alloc_dive();
 
@@ -924,6 +938,70 @@ static unsigned int fixup_suunto_versions(device_data_t *devdata, const dc_event
 
 	return serial;
 }
+#ifndef O_BINARY
+  #define O_BINARY 0
+#endif
+static void do_save_fingerprint(device_data_t *devdata, const char *tmp, const char *final)
+{
+	int fd, written;
+
+	fd = subsurface_open(tmp, O_WRONLY | O_BINARY | O_CREAT | O_TRUNC, 0666);
+	if (fd < 0)
+		return;
+	written = write(fd, devdata->fingerprint, devdata->fsize);
+	if (!close(fd) && written == devdata->fsize) {
+		if (subsurface_rename(tmp, final))
+			return;
+	}
+	unlink(tmp);
+}
+
+/*
+ * Save the fingerprint after a successful download
+ *
+ * NOTE! This should really only be done after the dive data has been
+ * accepted and saved, but there is no real sane way to do that.
+ */
+void save_fingerprint(device_data_t *devdata)
+{
+	char *dir, *tmp, *final;
+
+	if (!devdata->fingerprint)
+		return;
+
+	dir = format_string("%s/fingerprints", system_default_directory());
+	subsurface_mkdir(dir);
+	tmp = format_string("%s/%04x.tmp", dir, devdata->deviceid);
+	final = format_string("%s/%04x", dir, devdata->deviceid);
+	free(dir);
+
+	do_save_fingerprint(devdata, tmp, final);
+	free(tmp);
+	free(final);
+	free(devdata->fingerprint);
+	devdata->fingerprint = NULL;
+}
+
+/*
+ * Look up the fingerprint from the fingerprint caches, and
+ * give it to libdivecomputer to avoid downloading already
+ * downloaded dives.
+ */
+static void lookup_fingerprint(dc_device_t *device, device_data_t *devdata)
+{
+	char *cachename;
+	struct memblock mem;
+
+	if (devdata->force_download)
+		return;
+	cachename = format_string("%s/fingerprints/%04x",
+		system_default_directory(), devdata->deviceid);
+	if (readfile(cachename, &mem) > 0) {
+		dc_device_set_fingerprint(device, mem.buffer, mem.size);
+		free(mem.buffer);
+	}
+	free(cachename);
+}
 
 static void event_cb(dc_device_t *device, dc_event_type_t event, const void *data, void *userdata)
 {
@@ -964,6 +1042,7 @@ static void event_cb(dc_device_t *device, dc_event_type_t event, const void *dat
 				devinfo->firmware, devinfo->firmware,
 				devinfo->serial, devinfo->serial);
 		}
+
 		/*
 		 * libdivecomputer doesn't give serial numbers in the proper string form,
 		 * so we have to see if we can do some vendor-specific munging.
@@ -977,6 +1056,9 @@ static void event_cb(dc_device_t *device, dc_event_type_t event, const void *dat
 		 * DC_FIELD_STRING interface instead */
 		devdata->libdc_serial = devinfo->serial;
 		devdata->libdc_firmware = devinfo->firmware;
+
+		lookup_fingerprint(device, devdata);
+
 		break;
 	case DC_EVENT_CLOCK:
 		dev_info(devdata, translate("gettextFromC", "Event: systime=%" PRId64 ", devtime=%u\n"),
@@ -1159,6 +1241,8 @@ const char *do_libdivecomputer_import(device_data_t *data)
 	data->device = NULL;
 	data->context = NULL;
 	data->iostream = NULL;
+	data->fingerprint = NULL;
+	data->fsize = 0;
 
 	if (data->libdc_log && logfile_name)
 		fp = subsurface_fopen(logfile_name, "w");
@@ -1207,6 +1291,13 @@ const char *do_libdivecomputer_import(device_data_t *data)
 		fclose(fp);
 	}
 
+	/*
+	 * FIXME! This should be done by the downloader after the data has
+	 * been accepted and saved, because you won't be able to download
+	 * it again otherwise without forcing all dives to be downloaded.
+	 */
+	save_fingerprint(data);
+
 	return err;
 }
 
diff --git a/core/libdivecomputer.h b/core/libdivecomputer.h
index b9cc20132..c6a4d9ba4 100644
--- a/core/libdivecomputer.h
+++ b/core/libdivecomputer.h
@@ -25,6 +25,8 @@ typedef struct dc_user_device_t
 	dc_descriptor_t *descriptor;
 	const char *vendor, *product, *devname;
 	const char *model;
+	unsigned char *fingerprint;
+	unsigned int fsize;
 	uint32_t libdc_firmware, libdc_serial;
 	uint32_t deviceid, diveid;
 	dc_device_t *device;
_______________________________________________
subsurface mailing list
[email protected]
http://lists.subsurface-divelog.org/cgi-bin/mailman/listinfo/subsurface

Reply via email to