openxc/isotp-c

An implementation of the ISO-TP (ISO15765-2) CAN protocol in C

isotp-c's Issues

Returning C-array serious flaw

The function isotp_continue_receive returns a copy of the structure IsoTpMessage.
But this one also embeds a C-array (payload) that has been allocated within the function, so, on the stack.

Due to this, once the function returns, the C-array points to some freed stack memory, and then may present corrupted data.

I got the issue running ARM based chipset: using the callback is safe, but the returned message contains garbage data.

I 've no idea of how to solve this without changing the API.

Add an option to pad messages out to 8 bytes

Add support for multi-frame messages

Extended-Id messages sending only first 4 bytes as arbitration id is defined as uint16.

The arbitration_id is defined as uint16_t.

When using the isotp-c library in combination with the UDS library and sending extended UDS messages this results in the CAN id only showing the first four bytes.

Changing all the arbitration_id to uint32_t should solve the problem.

Travis CI fails

https://travis-ci.org/hackrid/isotp-c/builds/142710775

Obsolete struct initialization syntax

The code uses an obsolete GCC extension syntax for struct initialization.

Example:

IsoTpReceiveHandle handle = {
    success: false,
    completed: false,
    arbitration_id: arbitration_id,
    message_received_callback: callback
};

This syntax is obsolete since GCC 2.5 (released in 1993!) (source: Designated Inits, source: GCC releases)

There are a few alternatives to this. One of which is a standarized version of field initialization introduced in C99:

IsoTpReceiveHandle handle = {
    .success = false,
    .completed = false,
    .arbitration_id = arbitration_id,
    .message_received_callback = callback
};

C99 support is quite widespread (source) and even if declared "partial", designated initializers should be available on any major, modern compiler (however, this cross-compiler support doesn't matter, since the code relies on an ancient GCC extension anyway). And to ensure that on GCC it still works as before, a simple preprocessor macro might be prepared to choose between two syntaxes.

There are also other standard options of struct initialization, like using a factory function or setting fields manually. I believe that this would not introduce any performance hit (if that worried you), since there are very few places that use that syntax and in most of them we return the initialized struct immediately.

I might prepare a pull request for that change.

no such file #include <bitfield/bitfield.h>

no such file #include <bitfield/bitfield.h>