eclipse-wakaama / wakaama

Eclipse Wakaama is a C implementation of the Open Mobile Alliance's LightWeight M2M protocol (LWM2M).

BSD 3-Clause "New" or "Revised" License

497 stars 374 forks source link

Clean up Clang Static Analyzer findings #578

Open rettichschnidi opened 3 years ago

rettichschnidi commented 3 years ago

With PR #570 merged, someone should deal with all the findings and either fix or mark them as false positive. Once the count is at zero, we can start breaking the build if new issues get introduced.

Acceptance Criteria

Clang static analyzer does not find any issues anymore
Build breaks if any subsequent commits introduce new issues

tuve commented 3 years ago

I had a quick look at this and noticed that it seems to be findings of type "Dereference of null pointer" this i think have been up for discussion before and my recollection of that was that we (in order to keep the footprint down) didn't want to do null checks then a possible null value was caused by a programmer err. How should we handle those findings, or should we deviate from this and just fix them? @sbernard31 @qleisan @rettichschnidi @mlasch

rettichschnidi commented 3 years ago

The overhead for checking for NULL pointers is extremely low, I'd prefer do this always. This project is dealing with data coming from the network, so we should err on save side.

tuve commented 3 years ago

I agree, yes there is an overhead but, as with coding standards, any possible gain of "running your own" is so minute compared to the convenience of a tool and a standard.

sbernard31 commented 3 years ago

Again, I let the team decide on this. https://github.com/eclipse/wakaama/issues/513#issuecomment-763513438 sounds related to this point.