From 3b25a0491ce9ef9b55c903c6c7f0929bc2910d1a Mon Sep 17 00:00:00 2001 From: Christopher Peplin Date: Thu, 2 Jan 2014 17:14:56 -0500 Subject: Allocate ISO-TP message buffer on the stack. --- src/isotp/isotp.c | 21 ++++++++++++++++----- src/isotp/isotp.h | 11 +++++++---- src/isotp/send.c | 6 +++--- tests/common.c | 8 ++++---- tests/test_receive.c | 4 ++-- tests/test_send.c | 8 ++++---- 6 files changed, 36 insertions(+), 22 deletions(-) diff --git a/src/isotp/isotp.c b/src/isotp/isotp.c index 1605d435..67c644f1 100644 --- a/src/isotp/isotp.c +++ b/src/isotp/isotp.c @@ -2,8 +2,6 @@ #include #include -const uint16_t MAX_ISO_TP_MESSAGE_SIZE = 4096; -const uint16_t MAX_CAN_FRAME_SIZE = 8; const uint8_t ISO_TP_DEFAULT_RESPONSE_TIMEOUT = 100; const bool ISO_TP_DEFAULT_FRAME_PADDING_STATUS = true; @@ -24,10 +22,11 @@ IsoTpShims isotp_init_shims(LogShim log, SendCanMessageShim send_can_message, void isotp_message_to_string(const IsoTpMessage* message, char* destination, size_t destination_length) { char payload_string[message->size * 2 + 1]; + memset(payload_string, 0, sizeof(payload_string)); for(int i = 0; i < message->size; i++) { // TODO, bah this isn't working because snprintf hits the NULL char that // it wrote the last time and stops cold - snprintf(&payload_string[i * 2], 2, "%02x", message->payload[i]); + /* snprintf(&payload_string[i * 2], 2, "%02x", message->payload[i]); */ } snprintf(destination, destination_length, "ID: 0x%02x, Payload: 0x%s", message->arbitration_id, payload_string); @@ -38,7 +37,9 @@ IsoTpMessage isotp_receive_can_frame(IsoTpShims* shims, IsoTpHandle* handle, const uint8_t data_length) { IsoTpMessage message = { arbitration_id: arbitration_id, - completed: false + completed: false, + payload: {0}, + size: 0 }; if(data_length < 1) { @@ -47,10 +48,18 @@ IsoTpMessage isotp_receive_can_frame(IsoTpShims* shims, IsoTpHandle* handle, if(handle->type == ISOTP_HANDLE_RECEIVING) { if(handle->receive_handle.arbitration_id != arbitration_id) { + if(shims->log != NULL) { + shims->log("The arb ID 0x%x doesn't match the expected rx ID 0x%x", + arbitration_id, handle->receive_handle.arbitration_id); + } return message; } } else if(handle->type == ISOTP_HANDLE_SENDING) { if(handle->send_handle.receiving_arbitration_id != arbitration_id) { + if(shims->log != NULL) { + shims->log("The arb ID 0x%x doesn't match the expected tx continuation ID 0x%x", + arbitration_id, handle->send_handle.receiving_arbitration_id); + } return message; } } else { @@ -73,7 +82,9 @@ IsoTpMessage isotp_receive_can_frame(IsoTpShims* shims, IsoTpHandle* handle, switch(pci) { case PCI_SINGLE: { - message.payload = payload; + if(payload_length > 0) { + memcpy(message.payload, payload, payload_length); + } message.size = payload_length; message.completed = true; handle->success = true; diff --git a/src/isotp/isotp.h b/src/isotp/isotp.h index c0db0178..adf0f247 100644 --- a/src/isotp/isotp.h +++ b/src/isotp/isotp.h @@ -6,19 +6,22 @@ #include #define CAN_MESSAGE_BYTE_SIZE 8 +#define MAX_ISO_TP_MESSAGE_SIZE 4096 +// TODO we want to avoid malloc, and we can't be allocated 4K on the stack for +// each IsoTpMessage, so for now we're setting an artificial max message size +// here - since we only handle single frame messages, 8 bytes is plenty. +#define OUR_MAX_ISO_TP_MESSAGE_SIZE 8 #ifdef __cplusplus extern "C" { #endif -const uint16_t MAX_ISO_TP_MESSAGE_SIZE; -const uint16_t MAX_CAN_FRAME_SIZE; const uint8_t ISO_TP_DEFAULT_RESPONSE_TIMEOUT; const bool ISO_TP_DEFAULT_FRAME_PADDING_STATUS; typedef struct { const uint16_t arbitration_id; - uint8_t* payload; + uint8_t payload[OUR_MAX_ISO_TP_MESSAGE_SIZE]; uint16_t size; bool completed; } IsoTpMessage; @@ -121,7 +124,7 @@ void isotp_message_to_string(const IsoTpMessage* message, char* destination, size_t destination_length); IsoTpHandle isotp_send(IsoTpShims* shims, const uint16_t arbitration_id, - const uint8_t* payload, uint16_t size, + const uint8_t payload[], uint16_t size, IsoTpMessageSentHandler callback); IsoTpHandle isotp_receive(IsoTpShims* shims, diff --git a/src/isotp/send.c b/src/isotp/send.c index 85e35744..b87c5602 100644 --- a/src/isotp/send.c +++ b/src/isotp/send.c @@ -44,7 +44,7 @@ IsoTpHandle isotp_send_single_frame(IsoTpShims* shims, IsoTpMessage* message, IsoTpHandle isotp_send_multi_frame(IsoTpShims* shims, IsoTpMessage* message, IsoTpMessageSentHandler callback) { - // TODO make sure to copy payload into a local buffer + // TODO make sure to copy message into a local buffer shims->log("Only single frame messages are supported"); IsoTpHandle handle = { success: false, @@ -57,14 +57,14 @@ IsoTpHandle isotp_send_multi_frame(IsoTpShims* shims, IsoTpMessage* message, } IsoTpHandle isotp_send(IsoTpShims* shims, const uint16_t arbitration_id, - const uint8_t* payload, uint16_t size, + const uint8_t payload[], uint16_t size, IsoTpMessageSentHandler callback) { IsoTpMessage message = { arbitration_id: arbitration_id, - payload: payload, size: size }; + memcpy(message.payload, payload, size); if(size < 8) { return isotp_send_single_frame(shims, &message, callback); } else { diff --git a/tests/common.c b/tests/common.c index 247d8f3d..882c266a 100644 --- a/tests/common.c +++ b/tests/common.c @@ -14,12 +14,12 @@ bool can_frame_was_sent; bool message_was_received; uint16_t last_message_received_arb_id; -uint8_t* last_message_received_payload; +uint8_t last_message_received_payload[OUR_MAX_ISO_TP_MESSAGE_SIZE]; uint8_t last_message_received_payload_size; uint16_t last_message_sent_arb_id; bool last_message_sent_status; -uint8_t* last_message_sent_payload; +uint8_t last_message_sent_payload[OUR_MAX_ISO_TP_MESSAGE_SIZE]; uint8_t last_message_sent_payload_size; void debug(const char* format, ...) { @@ -84,8 +84,8 @@ void can_frame_sent(const uint16_t arbitration_id, const uint8_t* payload, void setup() { SHIMS = isotp_init_shims(debug, mock_send_can, mock_set_timer); HANDLE = isotp_receive(&SHIMS, 0x2a, message_received); - last_message_sent_payload = malloc(MAX_ISO_TP_MESSAGE_SIZE); - last_message_received_payload = malloc(MAX_ISO_TP_MESSAGE_SIZE); + memset(last_message_sent_payload, 0, OUR_MAX_ISO_TP_MESSAGE_SIZE); + memset(last_message_received_payload, 0, OUR_MAX_ISO_TP_MESSAGE_SIZE); memset(last_can_payload_sent, 0, sizeof(last_can_payload_sent)); last_message_sent_status = false; message_was_received = false; diff --git a/tests/test_receive.c b/tests/test_receive.c index 5c757076..d8c2392e 100644 --- a/tests/test_receive.c +++ b/tests/test_receive.c @@ -15,12 +15,12 @@ extern bool can_frame_was_sent; extern bool message_was_received; extern uint16_t last_message_received_arb_id; -extern uint8_t* last_message_received_payload; +extern uint8_t last_message_received_payload[]; extern uint8_t last_message_received_payload_size; extern uint16_t last_message_sent_arb_id; extern bool last_message_sent_status; -extern uint8_t* last_message_sent_payload; +extern uint8_t last_message_sent_payload[]; extern uint8_t last_message_sent_payload_size; extern void setup(); diff --git a/tests/test_send.c b/tests/test_send.c index 0320576c..ca1842a6 100644 --- a/tests/test_send.c +++ b/tests/test_send.c @@ -17,12 +17,12 @@ extern bool can_frame_was_sent; extern bool message_was_received; extern uint16_t last_message_received_arb_id; -extern uint8_t* last_message_received_payload; +extern uint8_t last_message_received_payload[]; extern uint8_t last_message_received_payload_size; extern uint16_t last_message_sent_arb_id; extern bool last_message_sent_status; -extern uint8_t* last_message_sent_payload; +extern uint8_t last_message_sent_payload[]; extern uint8_t last_message_sent_payload_size; extern void setup(); @@ -49,8 +49,8 @@ START_TEST (test_send_single_frame) { const uint8_t payload[] = {0x12, 0x34}; uint16_t arbitration_id = 0x2a; - IsoTpHandle handle = isotp_send(&SHIMS, arbitration_id, payload, sizeof(payload), - message_sent); + IsoTpHandle handle = isotp_send(&SHIMS, arbitration_id, payload, + sizeof(payload), message_sent); ck_assert_int_eq(last_message_sent_arb_id, arbitration_id); fail_unless(last_message_sent_status); ck_assert_int_eq(last_message_sent_payload[0], 0x12); -- cgit 1.2.3-korg