[#619] core: Fix segfault during observe error callback

[mlasch]

This PR fixes a NULL pointer access in prv_obsRequestCallback and prv_obsCancelRequestCallback which occurred when the requests timed out. The block transfer checks were re-ordered such that the pointer now is checked before it is accessed.

This should solve #619

[mlasch]

@tuve can you have a look at this PR?

[mlasch]

@shabunin feel free to review.

[shabunin]

@mlasch thank you for your work.
Indeed, no segfault anymore.
Tested two cases: first, when observe request timeout triggers before device monitoring callback 202_deleted and, second, when observe 500 error called after device was removed.
In both cases everything is fine and in second case I can freely deallocate userData for existing observations from deleted clientP because our troubled observation is not present in that list.

[shabunin]

Is there point to run callback with new observation data?

Mayhaps is better to run old callback of found observation with it's data?

observationP->callback(contextP, observationP->clientP->internalID, &observationP->uri,
                             COAP_202_DELETED,
                             NULL, LWM2M_CONTENT_TEXT, NULL, 0, observationP->userData);

I understand that yours change was only formatting, anyway

[mlasch]

Yes, this is formatting only as a result of running clang-format. Frankly, I am not a 100% sure how to reach this code. It looks like the callback is triggered in an edge case to be able to free observation userData in the callback, if you provided a pointer to userData on the heap when starting the observation with lwm2m_observe(...) (however, this is not used by the server example). I updated the PR to remove the "formatting only" changes to keep the history a bit cleaner.

[shabunin]

You can reach it by sending second obervation request for the same object.
New observation replaces the old one and old callback will not be called for every new observed value.

I assume previous userData should be freed in observe callback:

if (dataLength == 0 && status == COAP_202_DELETED) {
    free((mystruct_t *)userData);
}

Currently, if I do this, fresh memory allocation is destroyed, not the old on.

This is not related to #619, should I open new issue?

Thanks.

[mlasch]

Yes, we should track that in a new issue, so this can be closed.