Skip to content

Commit 423b52e

Browse files
SenRamakriSenRamakri
committed
Fix crc calculation error, code-style issues and other fixes
1 parent f1926c0 commit 423b52e

File tree

4 files changed

+85
-78
lines changed

4 files changed

+85
-78
lines changed

‎TESTS/mbed_platform/crash_reporting/main.cpp

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -29,11 +29,12 @@
2929
#define MSG_KEY_DEVICE_READY "crash_reporting_ready"
3030
#define MSG_KEY_DEVICE_ERROR "crash_reporting_inject_error"
3131

32-
void mbed_error_reboot_callback(mbed_error_ctx *error_context) {
32+
void mbed_error_reboot_callback(mbed_error_ctx *error_context)
33+
{
3334
TEST_ASSERT_EQUAL_UINT(MBED_ERROR_OUT_OF_MEMORY, error_context->error_status);
3435
TEST_ASSERT_EQUAL_UINT(1, error_context->error_reboot_count);
3536
mbed_reset_reboot_error_info();
36-
37+
3738
//Send the ready msg to host to indicate test pass
3839
greentea_send_kv(MSG_KEY_DEVICE_READY, MSG_VALUE_DUMMY);
3940
}
@@ -46,12 +47,15 @@ void test_crash_reporting()
4647
static char _key[MSG_KEY_LEN + 1] = { };
4748
static char _value[MSG_VALUE_LEN + 1] = { };
4849

50+
printf("\nWaiting for inject error\n");
4951
greentea_parse_kv(_key, _value, MSG_KEY_LEN, MSG_VALUE_LEN);
52+
printf("\nGot inject error\n");
5053
if (strcmp(_key, MSG_KEY_DEVICE_ERROR) == 0) {
54+
printf("\nErroring...\n");
5155
MBED_ERROR1(MBED_ERROR_OUT_OF_MEMORY, "Executing crash reporting test.", 0xDEADBAD);
5256
TEST_ASSERT_MESSAGE(0, "crash_reporting() error call failed.");
5357
}
54-
58+
printf("\nWaiting for inject error");
5559
TEST_ASSERT_MESSAGE(0, "Unexpected message received.");
5660
}
5761

‎platform/mbed_crash_data_offsets.h

Lines changed: 22 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -24,28 +24,28 @@ extern "C" {
2424
#endif
2525

2626
#if MBED_CONF_PLATFORM_CRASH_CAPTURE_ENABLED
27-
#if defined(__CC_ARM) || (defined(__ARMCC_VERSION) && (__ARMCC_VERSION >= 6010050))
28-
extern uint32_t Image$$RW_m_crash_data$$ZI$$Base[];
29-
extern uint32_t Image$$RW_m_crash_data$$ZI$$Size;
30-
#define __CRASH_DATA_RAM_START__ Image$$RW_m_crash_data$$ZI$$Base
31-
#define __CRASH_DATA_RAM_SIZE__ Image$$RW_m_crash_data$$ZI$$Size
32-
#elif defined(__ICCARM__)
33-
extern uint32_t __CRASH_DATA_RAM_START__[];
34-
extern uint32_t __CRASH_DATA_RAM_END__[];
35-
#define __CRASH_DATA_RAM_SIZE__ (__CRASH_DATA_RAM_END__ - __CRASH_DATA_RAM_START__)
36-
#elif defined(__GNUC__)
37-
extern uint32_t __CRASH_DATA_RAM_START__[];
38-
extern uint32_t __CRASH_DATA_RAM_END__[];
39-
#define __CRASH_DATA_RAM_SIZE__ (__CRASH_DATA_RAM_END__ - __CRASH_DATA_RAM_START__)
40-
#endif /* defined(__CC_ARM) */
41-
42-
/* Offset definitions for context capture */
43-
#define FAULT_CONTEXT_OFFSET (0x0)
44-
#define FAULT_CONTEXT_SIZE (0x80 / 4) //32 words(128 bytes) for Fault Context
45-
#define ERROR_CONTEXT_OFFSET (FAULT_CONTEXT_OFFSET + FAULT_CONTEXT_SIZE)
46-
#define ERROR_CONTEXT_SIZE (0x80 / 4) //32 words(128 bytes) bytes for Error Context
47-
#define FAULT_CONTEXT_LOCATION (__CRASH_DATA_RAM_START__ + FAULT_CONTEXT_OFFSET)
48-
#define ERROR_CONTEXT_LOCATION (__CRASH_DATA_RAM_START__ + ERROR_CONTEXT_OFFSET)
27+
#if defined(__CC_ARM) || (defined(__ARMCC_VERSION) && (__ARMCC_VERSION >= 6010050))
28+
extern uint32_t Image$$RW_m_crash_data$$ZI$$Base[];
29+
extern uint32_t Image$$RW_m_crash_data$$ZI$$Size;
30+
#define __CRASH_DATA_RAM_START__ Image$$RW_m_crash_data$$ZI$$Base
31+
#define __CRASH_DATA_RAM_SIZE__ Image$$RW_m_crash_data$$ZI$$Size
32+
#elif defined(__ICCARM__)
33+
extern uint32_t __CRASH_DATA_RAM_START__[];
34+
extern uint32_t __CRASH_DATA_RAM_END__[];
35+
#define __CRASH_DATA_RAM_SIZE__ (__CRASH_DATA_RAM_END__ - __CRASH_DATA_RAM_START__)
36+
#elif defined(__GNUC__)
37+
extern uint32_t __CRASH_DATA_RAM_START__[];
38+
extern uint32_t __CRASH_DATA_RAM_END__[];
39+
#define __CRASH_DATA_RAM_SIZE__ (__CRASH_DATA_RAM_END__ - __CRASH_DATA_RAM_START__)
40+
#endif /* defined(__CC_ARM) */
41+
42+
/* Offset definitions for context capture */
43+
#define FAULT_CONTEXT_OFFSET (0x0)
44+
#define FAULT_CONTEXT_SIZE (0x80 / 4) //32 words(128 bytes) for Fault Context
45+
#define ERROR_CONTEXT_OFFSET (FAULT_CONTEXT_OFFSET + FAULT_CONTEXT_SIZE)
46+
#define ERROR_CONTEXT_SIZE (0x80 / 4) //32 words(128 bytes) bytes for Error Context
47+
#define FAULT_CONTEXT_LOCATION (__CRASH_DATA_RAM_START__ + FAULT_CONTEXT_OFFSET)
48+
#define ERROR_CONTEXT_LOCATION (__CRASH_DATA_RAM_START__ + ERROR_CONTEXT_OFFSET)
4949
#endif
5050

5151
#ifdef __cplusplus

‎platform/mbed_error.c

Lines changed: 47 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@
4040
#define ERROR_REPORT(ctx, error_msg, error_filename, error_line, print_thread_info) print_error_report(ctx, error_msg, error_filename, error_line, print_thread_info)
4141
static void print_error_report(const mbed_error_ctx *ctx, const char *, const char *error_filename, int error_line, bool print_thread_info);
4242
#else
43-
#define ERROR_REPORT(ctx, error_msg, error_filename, error_line) ((void) 0)
43+
#define ERROR_REPORT(ctx, error_msg, error_filename, error_line, print_thread_info) ((void) 0)
4444
#endif
4545

4646
static core_util_atomic_flag error_in_progress = CORE_UTIL_ATOMIC_FLAG_INIT;
@@ -49,9 +49,9 @@ static int error_count = 0;
4949
static mbed_error_ctx first_error_ctx = {0};
5050

5151
#if MBED_CONF_PLATFORM_CRASH_CAPTURE_ENABLED
52-
//Global for populating the context in exception handler
53-
static mbed_error_ctx *const report_error_ctx=(mbed_error_ctx *)(ERROR_CONTEXT_LOCATION);
54-
static bool is_reboot_error_valid = false;
52+
//Global for populating the context in exception handler
53+
static mbed_error_ctx *const report_error_ctx = (mbed_error_ctx *)(ERROR_CONTEXT_LOCATION);
54+
static bool is_reboot_error_valid = false;
5555
#endif
5656

5757
static mbed_error_ctx last_error_ctx = {0};
@@ -61,19 +61,20 @@ static mbed_error_status_t handle_error(mbed_error_status_t error_status, unsign
6161
//Helper function to calculate CRC
6262
//NOTE: It would have been better to use MbedCRC implementation. But
6363
//MbedCRC uses table based calculation and we dont want to keep that table memory
64-
//used up for this purpose. Also we cannot force bitwise calculation in MbedCRC
64+
//used up for this purpose. Also we cannot force bitwise calculation in MbedCRC
6565
//and it also requires a new wrapper to be called from C implementation. Since
6666
//we dont have many uses cases to create a C wrapper for MbedCRC and the data
67-
//we calculate CRC on in this context is very less we will use a local
67+
//we calculate CRC on in this context is very less we will use a local
6868
//implementation here.
69-
static unsigned int compute_crc32(void *data, int datalen)
69+
static unsigned int compute_crc32(const void *data, int datalen)
7070
{
7171
const unsigned int polynomial = 0x04C11DB7; /* divisor is 32bit */
7272
unsigned int crc = 0; /* CRC value is 32bit */
73-
74-
for ( ;datalen>=0; datalen-- ) {
75-
unsigned char b = (*(unsigned char *)data);
76-
crc ^= (unsigned int )(b << 24); /* move byte into upper 8bit */
73+
unsigned char *buf = (unsigned char *)data;//use a temp variable to make code readable and to avoid typecasting issues.
74+
75+
for (; datalen>0; datalen-- ) {
76+
unsigned char b = *buf++;
77+
crc ^= (unsigned int)(b << 24); /* move byte into upper 8bit */
7778
for (int i = 0; i < 8; i++) {
7879
/* is MSB 1 */
7980
if ((crc & 0x80000000) != 0) {
@@ -191,7 +192,8 @@ static mbed_error_status_t handle_error(mbed_error_status_t error_status, unsign
191192
return MBED_SUCCESS;
192193
}
193194

194-
WEAK void mbed_error_reboot_callback(mbed_error_ctx *error_context) {
195+
WEAK void mbed_error_reboot_callback(mbed_error_ctx *error_context)
196+
{
195197
//Dont do anything here, let application override this if required.
196198
}
197199

@@ -200,32 +202,35 @@ mbed_error_status_t mbed_error_initialize(void)
200202
{
201203
#if MBED_CONF_PLATFORM_CRASH_CAPTURE_ENABLED
202204
uint32_t crc_val = 0;
203-
crc_val = compute_crc32( report_error_ctx, offsetof(mbed_error_ctx, crc_error_ctx) );
204-
//Read report_error_ctx and check if CRC is correct, and with valid status code
205-
if ((report_error_ctx->crc_error_ctx == crc_val) && (report_error_ctx->is_error_processed == 0) && (report_error_ctx->error_status < 0)) {
206-
is_reboot_error_valid = true;
207-
#if MBED_CONF_PLATFORM_REBOOT_CRASH_REPORT_ENABLED && !defined(NDEBUG)
208-
//Report the error info
209-
mbed_error_printf("\n== Your last reboot was triggered by an error, below is the error information ==");
210-
ERROR_REPORT( report_error_ctx, "System rebooted due to fatal error", MBED_FILENAME, __LINE__, false );
211-
#endif
212-
//Call the mbed_error_reboot_callback, this enables applications to do some handling before we do the handling
213-
mbed_error_reboot_callback(report_error_ctx);
205+
206+
//Just check if we have valid value for error_status, if error_status is positive(which is not valid), no need to check crc
207+
if (report_error_ctx->error_status < 0) {
208+
crc_val = compute_crc32(report_error_ctx, offsetof(mbed_error_ctx, crc_error_ctx));
209+
//Read report_error_ctx and check if CRC is correct, and with valid status code
210+
if ((report_error_ctx->crc_error_ctx == crc_val) && (report_error_ctx->is_error_processed == 0)) {
211+
is_reboot_error_valid = true;
212+
#if MBED_CONF_PLATFORM_REBOOT_CRASH_REPORT_ENABLED && !defined(NDEBUG)
213+
//Report the error info
214+
mbed_error_printf("\n== Your last reboot was triggered by an error, below is the error information ==");
215+
ERROR_REPORT(report_error_ctx, "System rebooted due to fatal error", MBED_FILENAME, __LINE__, false);
216+
#endif
217+
//Call the mbed_error_reboot_callback, this enables applications to do some handling before we do the handling
218+
mbed_error_reboot_callback(report_error_ctx);
214219

215-
//Enforce max-reboot only if auto reboot is enabled
216-
#if MBED_CONF_PLATFORM_FATAL_ERROR_AUTO_REBOOT_ENABLED
217-
if ( report_error_ctx->error_reboot_count >= MBED_CONF_PLATFORM_ERROR_REBOOT_MAX ) {
218-
//We have rebooted more than enough, hold the system here.
219-
mbed_error_printf("\n== Reboot count(=%ld) exceeded maximum, system halting ==\n", report_error_ctx->error_reboot_count);
220-
mbed_halt_system();
220+
//Enforce max-reboot only if auto reboot is enabled
221+
#if MBED_CONF_PLATFORM_FATAL_ERROR_AUTO_REBOOT_ENABLED
222+
if (report_error_ctx->error_reboot_count >= MBED_CONF_PLATFORM_ERROR_REBOOT_MAX) {
223+
//We have rebooted more than enough, hold the system here.
224+
mbed_error_printf("\n== Reboot count(=%ld) exceeded maximum, system halting ==\n", report_error_ctx->error_reboot_count);
225+
mbed_halt_system();
226+
}
227+
#endif
228+
report_error_ctx->is_error_processed = 1;//Set the flag that we already processed this error
229+
crc_val = compute_crc32(report_error_ctx, offsetof(mbed_error_ctx, crc_error_ctx));
230+
report_error_ctx->crc_error_ctx = crc_val;
221231
}
222-
#endif
223-
report_error_ctx->is_error_processed = 1;//Set the flag that we already processed this error
224-
crc_val = compute_crc32( report_error_ctx, offsetof(mbed_error_ctx, crc_error_ctx) );
225-
report_error_ctx->crc_error_ctx = crc_val;
226232
}
227233
#endif
228-
229234
return MBED_SUCCESS;
230235
}
231236

@@ -267,10 +272,10 @@ WEAK MBED_NORETURN mbed_error_status_t mbed_error(mbed_error_status_t error_stat
267272
//On fatal errors print the error context/report
268273
ERROR_REPORT(&last_error_ctx, error_msg, filename, line_number, true);
269274
}
270-
275+
271276
#if MBED_CONF_PLATFORM_CRASH_CAPTURE_ENABLED
272277
uint32_t crc_val = 0;
273-
crc_val = compute_crc32( report_error_ctx, offsetof(mbed_error_ctx, crc_error_ctx) );
278+
crc_val = compute_crc32(report_error_ctx, offsetof(mbed_error_ctx, crc_error_ctx));
274279
//Read report_error_ctx and check if CRC is correct for report_error_ctx
275280
if (report_error_ctx->crc_error_ctx == crc_val) {
276281
uint32_t current_reboot_count = report_error_ctx->error_reboot_count;
@@ -280,7 +285,7 @@ WEAK MBED_NORETURN mbed_error_status_t mbed_error(mbed_error_status_t error_stat
280285
}
281286
last_error_ctx.is_error_processed = 0;//Set the flag that this is a new error
282287
//Update the struct with crc
283-
last_error_ctx.crc_error_ctx = compute_crc32( &last_error_ctx, offsetof(mbed_error_ctx, crc_error_ctx) );
288+
last_error_ctx.crc_error_ctx = compute_crc32(&last_error_ctx, offsetof(mbed_error_ctx, crc_error_ctx));
284289
//Protect report_error_ctx while we update it
285290
core_util_critical_section_enter();
286291
memcpy(report_error_ctx, &last_error_ctx, sizeof(mbed_error_ctx));
@@ -291,8 +296,6 @@ WEAK MBED_NORETURN mbed_error_status_t mbed_error(mbed_error_status_t error_stat
291296
#endif
292297
#endif
293298
mbed_halt_system();
294-
295-
return MBED_ERROR_FAILED_OPERATION;
296299
}
297300

298301
//Register an application defined callback with error handling
@@ -313,10 +316,10 @@ mbed_error_status_t mbed_reset_reboot_error_info()
313316
#if MBED_CONF_PLATFORM_CRASH_CAPTURE_ENABLED
314317
//Protect for thread safety
315318
core_util_critical_section_enter();
316-
memset(report_error_ctx, 0, sizeof(mbed_error_ctx) );
319+
memset(report_error_ctx, 0, sizeof(mbed_error_ctx));
317320
core_util_critical_section_exit();
318321
#endif
319-
return MBED_SUCCESS;
322+
return MBED_SUCCESS;
320323
}
321324

322325
//Reset the reboot error context
@@ -328,13 +331,13 @@ mbed_error_status_t mbed_reset_reboot_count()
328331
core_util_critical_section_enter();
329332
report_error_ctx->error_reboot_count = 0;//Set reboot count to 0
330333
//Update CRC
331-
crc_val = compute_crc32( report_error_ctx, offsetof(mbed_error_ctx, crc_error_ctx) );
334+
crc_val = compute_crc32(report_error_ctx, offsetof(mbed_error_ctx, crc_error_ctx));
332335
report_error_ctx->crc_error_ctx = crc_val;
333336
core_util_critical_section_exit();
334337
return MBED_SUCCESS;
335338
}
336339
#endif
337-
return MBED_ERROR_ITEM_NOT_FOUND;
340+
return MBED_ERROR_ITEM_NOT_FOUND;
338341
}
339342

340343
//Retrieve the reboot error context
@@ -350,7 +353,7 @@ mbed_error_status_t mbed_get_reboot_error_info(mbed_error_ctx *error_info)
350353
status = MBED_ERROR_INVALID_ARGUMENT;
351354
}
352355
}
353-
#endif
356+
#endif
354357
return status;
355358
}
356359

‎platform/mbed_error.h

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -831,7 +831,7 @@ typedef struct _mbed_error_ctx {
831831
int32_t error_reboot_count;//everytime we write this struct we increment this value by 1, irrespective of time between reboots. Note that the data itself might change, but everytime we reboot due to error we update this count by 1
832832
int32_t is_error_processed;//once this error is processed set this value to 1
833833
uint32_t crc_error_ctx;//crc_error_ctx should always be the last member in this struct
834-
#endif
834+
#endif
835835
} mbed_error_ctx;
836836

837837
/** To generate a fatal compile-time error, you can use the pre-processor #error directive.
@@ -935,10 +935,10 @@ typedef void (*mbed_error_hook_t)(const mbed_error_ctx *error_ctx);
935935

936936

937937
/**
938-
* Callback function for reporting error context during boot up. When MbedOS error handling system detects a fatal error
938+
* Callback function for reporting error context during boot up. When MbedOS error handling system detects a fatal error
939939
* it will auto-reboot the system(if MBED_CONF_PLATFORM_FATAL_ERROR_AUTO_REBOOT_ENABLED is enabled) after capturing the
940-
* error info in special crash data RAM region. Once rebooted, MbedOS initialization routines will call this function with a pointer to
941-
* the captured mbed_error_ctx structure. If application implementation needs to receive this callback, mbed_error_reboot_callback
940+
* error info in special crash data RAM region. Once rebooted, MbedOS initialization routines will call this function with a pointer to
941+
* the captured mbed_error_ctx structure. If application implementation needs to receive this callback, mbed_error_reboot_callback
942942
* function should be overriden with custom implementation. By default it's defined as a WEAK function in mbed_error.c.
943943
* Note that this callback will be invoked before the system starts executing main() function. So the implementation of
944944
* the callback should be aware any resource limitations/availability of resources which are yet to be initialized by application main().
@@ -950,10 +950,10 @@ typedef void (*mbed_error_hook_t)(const mbed_error_ctx *error_ctx);
950950
void mbed_error_reboot_callback(mbed_error_ctx *error_context);
951951

952952
/**
953-
* Initialize error handling system, this is called by the mbed-os boot sequence. This is not required to be called by Application unless the boot sequence is overridden by the system implementation.
953+
* Initialize error handling system, this is called by the mbed-os boot sequence. This is not required to be called by Application unless the boot sequence is overridden by the system implementation.
954954
* NOTE: This function also prints the error report to serial terminal if MBED_CONF_PLATFORM_REBOOT_CRASH_REPORT_ENABLED is enabled.
955955
* If MBED_CONF_PLATFORM_FATAL_ERROR_AUTO_REBOOT_ENABLED is enabled and if the current reboot count exceeds MBED_CONF_PLATFORM_ERROR_REBOOT_MAX the system will halt when this function is called,
956-
* and in such cases the caller will not get the control back. Also note that calling this function may trigger mbed_error_reboot_callback() if application side overides mbed_error_reboot_callback().
956+
* and in such cases the caller will not get the control back. Also note that calling this function may trigger mbed_error_reboot_callback() if application side overides mbed_error_reboot_callback().
957957
* @return MBED_SUCCESS on success.
958958
*
959959
*/
@@ -965,21 +965,21 @@ mbed_error_status_t mbed_error_initialize(void);
965965
* @param error_info Pointer to mbed_error_ctx struct allocated by the caller. This is the mbed_error_ctx info captured as part of the fatal error which triggered the reboot.
966966
* @return 0 or MBED_SUCCESS on success.
967967
* MBED_ERROR_INVALID_ARGUMENT in case of invalid error_info pointer
968-
* MBED_ERROR_ITEM_NOT_FOUND if no reboot context is currently captured by the system
968+
* MBED_ERROR_ITEM_NOT_FOUND if no reboot context is currently captured by the system
969969
*
970970
*/
971971
mbed_error_status_t mbed_get_reboot_error_info(mbed_error_ctx *error_info);
972972

973973
/**
974-
* Calling this function resets the current reboot context captured by the system(stored in special crash data RAM region).
974+
* Calling this function resets the current reboot context captured by the system(stored in special crash data RAM region).
975975
* @return MBED_SUCCESS on success.
976976
* MBED_ERROR_ITEM_NOT_FOUND if no reboot context is currently captured by the system
977977
*/
978978
mbed_error_status_t mbed_reset_reboot_error_info(void);
979979

980980
/**
981981
* Calling this function resets the current reboot count stored as part of error context captured in special crash data RAM region.
982-
* The function will also update the CRC value stored as part of error context accordingly.
982+
* The function will also update the CRC value stored as part of error context accordingly.
983983
* @return MBED_SUCCESS on success.
984984
* MBED_ERROR_ITEM_NOT_FOUND if no reboot context is currently captured by the system
985985
*/

0 commit comments

Comments
 (0)