diff options
author | Linus Torvalds <torvalds@linux-foundation.org> | 2019-09-11 15:27:29 +0100 |
---|---|---|
committer | Dirk Hohndel <dirk@hohndel.org> | 2019-09-11 21:51:39 +0100 |
commit | 958dce33263ecda91fae0f5d25494ec2ca5808ba (patch) | |
tree | e6909dbc8926d66b737a11467b4d18564675c7cb /core/libdivecomputer.c | |
parent | cd194332907652df6892235c21c8ca7ab1f1af7e (diff) | |
download | subsurface-958dce33263ecda91fae0f5d25494ec2ca5808ba.tar.gz |
Keep parsing dives even if one dive parse failed
Eric Charbonnier reported a problem downloading the dives from his
OSTC2, and Jef debugged the libdivecomputer log and says:
"Your ostc has 75 dives, but subsurface downloaded only one, and then
stopped the download. That's because that first dive appears to be
corrupt and fails to parse:
ERROR: Buffer overflow detected! [in /win/subsurface/libdivecomputer/src/hw_ostc_parser.c:981 (hw_ostc_parser_samples_foreach)]
Subsurface (incorrectly) considers that a fatal error and stops the
entire download. From a user point of view, it would be much better to
ignore the problematic dive, and continue downloading the remaining"
Subsurface used to just stop downloading if there were parsing errors,
but Jef further says:
"How parser errors are handled is up to the application. Aborting the
download is probably the worst option here. If a dive fails to parse
(because the dive data is corrupt, the parser contains a bug, etc),
that does not necessary mean the remaining dives can't be downloaded"
so let's change the logic to just continue downloading, and hope other
dives work better.
We might want to do better error reporting, right now the errors tend to
just cause "dev_info()" reports, which just set the progress bar text.
So you'll see it in the progress bar as it happens, but it won't get
really ever noted as an error, and it's easy to miss.
But that error reporting is a separate issue, and this just does the
"continue to the next dive" part.
Reported-by: Eric Charbonnier <eric.charbonnier69@gmail.com>
Suggested-by: Jef Driesen <jef@libdivecomputer.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Diffstat (limited to 'core/libdivecomputer.c')
-rw-r--r-- | core/libdivecomputer.c | 11 |
1 files changed, 6 insertions, 5 deletions
diff --git a/core/libdivecomputer.c b/core/libdivecomputer.c index 2e4441933..47480a5b4 100644 --- a/core/libdivecomputer.c +++ b/core/libdivecomputer.c @@ -770,7 +770,7 @@ static int dive_cb(const unsigned char *data, unsigned int size, rc = create_parser(devdata, &parser); if (rc != DC_STATUS_SUCCESS) { dev_info(devdata, translate("gettextFromC", "Unable to create parser for %s %s"), devdata->vendor, devdata->product); - return false; + return true; } rc = dc_parser_set_data(parser, data, size); @@ -810,16 +810,17 @@ static int dive_cb(const unsigned char *data, unsigned int size, goto error_exit; } + dc_parser_destroy(parser); + /* If we already saw this dive, abort. */ if (!devdata->force_download && find_dive(&dive->dc)) { char *date_string = get_dive_date_c_string(dive->when); dev_info(devdata, translate("gettextFromC", "Already downloaded dive at %s"), date_string); free(date_string); - goto error_exit; + free(dive); + return false; } - dc_parser_destroy(parser); - /* Various libdivecomputer interface fixups */ if (dive->dc.airtemp.mkelvin == 0 && first_temp_is_air && dive->dc.samples) { dive->dc.airtemp = dive->dc.sample[0].temperature; @@ -832,7 +833,7 @@ static int dive_cb(const unsigned char *data, unsigned int size, error_exit: dc_parser_destroy(parser); free(dive); - return false; + return true; } |