question about the code

[Dirk Hohndel]

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