devel December 2017

devel@libdivecomputer.org

5 participants
4 discussions

Permanent mirror of repository on GitHub?
by Henrik B A 05 Jan '18

05 Jan '18

Hi Jef. Would you consider creating a permanent & updated mirror of libdevcomputer on GitHub? I like browsing the source code in a browser, and GitHub is such a good tool for doing that. The web interface at http://git.libdivecomputer.org/?p=libdivecomputer.git;a=summary is slow, times out a lot, and is really not very usable. Cheers, Henrik

4 7

Coverity / static analysis based cleanups
by Dirk Hohndel 03 Jan '18

03 Jan '18

This is a series of 14 patches against the current tip of the Subsurface-branch of libdivecomputer. I believe that all of them should apply to upstream master (but haven't verified that). Some of them are clear bug fixes, some of them are equally clear fixes to avoid memory leaks. A few deal with cases where the current code ignores the return values of functions - that last type I am not always 100% sure about my resolution... All of them were generated based on a Coverity scan of Subsurface, built on Linux. This series should address all of the currently open Coverity issues that aren't rather obvious false positives. As usual, one can argue whether all of these issues are indeed bugs and how they should be fixed (e.g., should we check return values and return errors to our callers, or is it safe to ignore certain errors (in which case that should be done explicitly for consistency)). Happy to get feedback / comments. Thanks /D [PATCH 01/14] Cleanup: correctly handle upper bound of array [PATCH 02/14] Cleanup: avoid memory leaks [PATCH 03/14] Cleanup: avoid memory leak [PATCH 04/14] Cleanup: avoid memory leak [PATCH 05/14] Cleanup: avoid memory leak [PATCH 06/14] Cleanup: ensure string is 0 terminated [PATCH 07/14] Cleanup: avoid undefined shift operation [PATCH 08/14] Cleanup: remove dead code and return the correct return [PATCH 09/14] Cleanup: be explicit about ignoring the return value of [PATCH 10/14] Cleanup: check return value of ioctl() [PATCH 11/14] Cleanup: check error return values of buffer handling [PATCH 12/14] Cleanup: consistenty check return value of iostream [PATCH 13/14] Cleanup: bail on error [PATCH 14/14] Cleanup: remove confusing NULL check

3 28

question about the code
by Dirk Hohndel 29 Dec '17

29 Dec '17

Hi Jef, I'm in the middle of addressing some issues that static code analysis has uncovered in the libdivecomputer code. One of the things pointed out by Coverity is that this piece of code is pretty non-sensical (uwatec_smart.c around line 222): static dc_status_t uwatec_smart_device_close (dc_device_t *abstract) { dc_status_t status = DC_STATUS_SUCCESS; uwatec_smart_device_t *device = (uwatec_smart_device_t*) abstract; dc_status_t rc = DC_STATUS_SUCCESS; // Close the device. rc = dc_iostream_close (device->iostream); if (status != DC_STATUS_SUCCESS) { dc_status_set_error(&status, rc); } return status; } So we set status to DC_STATUS_SUCCESS Then we set rc to DC_STATUS_SUCCESS Then we immediately overwrite rc with the return value of dc_iostream_close Then we test status(!) for being different from DC_STATUS_SUCCESS (which of course it never is as we explicitly set it to that value). Inside that we call a helper function that overwites status, but only if it is not equal to DC_STATUS_SUCCESS And then we return - ta-da! - status. What I /think/ you wanted to write was this: static dc_status_t uwatec_smart_device_close (dc_device_t *abstract) { uwatec_smart_device_t *device = (uwatec_smart_device_t*) abstract; // Close the device and pass up the return code return dc_iostream_close (device->iostream); } Am I missing something here? A patch that fixes this will be included in my patches later today. /D

2 2

Subsurface libdivecomputer branch merged with upstream..
by Linus Torvalds 13 Dec '17

13 Dec '17

So this took many hours and much frustration, but I've got an updated libdivecomputer tree at https://github.com/torvalds/libdc-for-dirk which has the iostream code from Jef's upstream libdivecomputer. I tried very hard to actually make use of the new iostream code, and in fact it does help get rid of one particularly ugly hack of ours: we used to hook into the serial code with a particularly ugly little hack using the RETURN_IF_CUSTOM_SERIAL() macro. We now avoid that, and actually create our own custom iostream for that case, and then the new iostream code does things right. HOWEVER. We don't actually use Jef's "custom" iostream layer for it, and most particularly, we don't actually expose that to the outside. The actual outside interface remains our trusty old "dc_custom_io_t", and the special custom serial case is actually just a trivial wrapper inside libdivecomputer for that. It's only used for the legacy serial emulation (ie notably Shearwater and OSTC). The Scubapro G2 and EON Steel end up using the subsurface dc_custom_io_t model directly, since for them it's not just a serial stream. End result: it all looks ok. I've tested it here with my EON Steel (both BLE and USB), my Scubapro G2 (also BLE and USB) and my Shearwater Perdix AI (BLE only, obviously). But I only pushed to my own "libdc-for-disk" repository, because this is a *big* change, and I'd like at least Dirk to check that it works for his cases too. Jef: I basically ended up using the "custom" approach for just the internal serial emulation. I *considered* just changing your "custom.c" file to make that work for me, but ended up just instead adding a "src/custom_io.c" file instead. It's pretty close to your custom.c file, but it is explicitly designed to interface with our "dc_custom_io_t". As mentioned elsewhere, I don't think your custom implementation actually is usable for outsiders. Dirk - caveat emptor. I think it's all good. It looks sane. It passes my tests. But the diffs to both of the parents in that merge is complicated, and the merge does a fair amount of things that were not in either parent since it only made sense as part of the merged state. I'm actually fairly happy with the end result, but the merge itself was painful enough that you really should check it out. Linus

2 2

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

2010

devel December 2017