From 026e5cc9c57ca1babd4d32ba01de45d4eaf40ea3 Mon Sep 17 00:00:00 2001 From: "Ben V. Brown" Date: Sun, 2 Aug 2020 16:18:43 +1000 Subject: [PATCH] NULL pointer checks for race --- .../TS100/Core/Drivers/FUSB302/hard_reset.cpp | 13 +++++--- .../TS100/Core/Drivers/FUSB302/int_n.cpp | 15 +++++---- workspace/TS100/Core/Drivers/FUSB302/int_n.h | 2 +- .../Core/Drivers/FUSB302/policy_engine.cpp | 32 +++++++------------ .../Drivers/FUSB302/policy_engine_user.cpp | 12 ++----- .../Core/Drivers/FUSB302/protocol_rx.cpp | 17 ++++++---- .../Core/Drivers/FUSB302/protocol_tx.cpp | 31 ++++++++++++------ 7 files changed, 64 insertions(+), 58 deletions(-) diff --git a/workspace/TS100/Core/Drivers/FUSB302/hard_reset.cpp b/workspace/TS100/Core/Drivers/FUSB302/hard_reset.cpp index 07426f95..97ffdd71 100644 --- a/workspace/TS100/Core/Drivers/FUSB302/hard_reset.cpp +++ b/workspace/TS100/Core/Drivers/FUSB302/hard_reset.cpp @@ -23,7 +23,7 @@ #include "protocol_tx.h" #include "fusb302b.h" -osThreadId ResetHandler::TaskHandle; +osThreadId ResetHandler::TaskHandle = NULL; uint32_t ResetHandler::TaskBuffer[ResetHandler::TaskStackSize]; osStaticThreadDef_t ResetHandler::TaskControlBlock; @@ -39,7 +39,8 @@ ResetHandler::hardrst_state ResetHandler::hardrst_reset_layer() { ProtocolReceive::notify( PDB_EVT_PRLRX_RESET); taskYIELD(); /* Reset the Protocol TX machine */ - ProtocolTransmit::notify( ProtocolTransmit::Notifications::PDB_EVT_PRLTX_RESET); + ProtocolTransmit::notify( + ProtocolTransmit::Notifications::PDB_EVT_PRLTX_RESET); taskYIELD(); /* Continue the process based on what event started the reset. */ if (evt & PDB_EVT_HARDRST_RESET) { @@ -97,13 +98,15 @@ ResetHandler::hardrst_state ResetHandler::hardrst_complete() { } void ResetHandler::init() { - osThreadStaticDef(rstHand, Thread, PDB_PRIO_PRL, 0, TaskStackSize, TaskBuffer, - &TaskControlBlock); + osThreadStaticDef(rstHand, Thread, PDB_PRIO_PRL, 0, TaskStackSize, + TaskBuffer, &TaskControlBlock); TaskHandle = osThreadCreate(osThread(rstHand), NULL); } void ResetHandler::notify(uint32_t notification) { - xTaskNotify(TaskHandle, notification, eNotifyAction::eSetBits); + if (TaskHandle != NULL) { + xTaskNotify(TaskHandle, notification, eNotifyAction::eSetBits); + } } void ResetHandler::Thread(const void *arg) { diff --git a/workspace/TS100/Core/Drivers/FUSB302/int_n.cpp b/workspace/TS100/Core/Drivers/FUSB302/int_n.cpp index 4e3dad92..52d7701b 100644 --- a/workspace/TS100/Core/Drivers/FUSB302/int_n.cpp +++ b/workspace/TS100/Core/Drivers/FUSB302/int_n.cpp @@ -28,7 +28,7 @@ #include "task.h" #include "BSP.h" -osThreadId InterruptHandler::TaskHandle; +osThreadId InterruptHandler::TaskHandle=NULL; uint32_t InterruptHandler::TaskBuffer[InterruptHandler::TaskStackSize]; osStaticThreadDef_t InterruptHandler::TaskControlBlock; @@ -46,9 +46,9 @@ void InterruptHandler::Thread(const void *arg) { while (true) { /* If the INT_N line is low */ if (xTaskNotifyWait(0x00, 0x0F, NULL, - PolicyEngine::setupCompleteOrTimedOut() ? 1000 : 200) == pdPASS) { + PolicyEngine::setupCompleteOrTimedOut() ? 1000 : 10) == pdPASS) { //delay slightly so we catch the crc with better timing - osDelay(2); + osDelay(1); } notifSent = false; /* Read the FUSB302B status and interrupt registers */ @@ -99,7 +99,10 @@ void InterruptHandler::Thread(const void *arg) { } } void InterruptHandler::irqCallback() { - BaseType_t taskWoke = pdFALSE; - xTaskNotifyFromISR(TaskHandle, 0x01, eNotifyAction::eSetBits, &taskWoke); - portYIELD_FROM_ISR(taskWoke); + if (TaskHandle != NULL) { + BaseType_t taskWoke = pdFALSE; + xTaskNotifyFromISR(TaskHandle, 0x01, eNotifyAction::eSetBits, + &taskWoke); + portYIELD_FROM_ISR(taskWoke); + } } diff --git a/workspace/TS100/Core/Drivers/FUSB302/int_n.h b/workspace/TS100/Core/Drivers/FUSB302/int_n.h index e7af6a4c..69c460f7 100644 --- a/workspace/TS100/Core/Drivers/FUSB302/int_n.h +++ b/workspace/TS100/Core/Drivers/FUSB302/int_n.h @@ -30,7 +30,7 @@ public: private: static void Thread(const void *arg); static osThreadId TaskHandle; - static const size_t TaskStackSize = 1536 / 4; + static const size_t TaskStackSize = 1536 / 3; static uint32_t TaskBuffer[TaskStackSize]; static osStaticThreadDef_t TaskControlBlock; /* diff --git a/workspace/TS100/Core/Drivers/FUSB302/policy_engine.cpp b/workspace/TS100/Core/Drivers/FUSB302/policy_engine.cpp index d015dd2a..0e8464fc 100644 --- a/workspace/TS100/Core/Drivers/FUSB302/policy_engine.cpp +++ b/workspace/TS100/Core/Drivers/FUSB302/policy_engine.cpp @@ -33,7 +33,7 @@ int8_t PolicyEngine::_hard_reset_counter; int8_t PolicyEngine::_old_tcc_match; uint8_t PolicyEngine::_pps_index; uint8_t PolicyEngine::_last_pps; -osThreadId PolicyEngine::TaskHandle; +osThreadId PolicyEngine::TaskHandle = NULL; uint32_t PolicyEngine::TaskBuffer[PolicyEngine::TaskStackSize]; osStaticThreadDef_t PolicyEngine::TaskControlBlock; union pd_msg PolicyEngine::tempMessage; @@ -42,8 +42,8 @@ PolicyEngine::policy_engine_state PolicyEngine::state = PESinkStartup; StaticQueue_t PolicyEngine::xStaticQueue; uint8_t PolicyEngine::ucQueueStorageArea[PDB_MSG_POOL_SIZE * sizeof(union pd_msg)]; -QueueHandle_t PolicyEngine::messagesWaiting; -EventGroupHandle_t PolicyEngine::xEventGroupHandle; +QueueHandle_t PolicyEngine::messagesWaiting = NULL; +EventGroupHandle_t PolicyEngine::xEventGroupHandle = NULL; StaticEventGroup_t PolicyEngine::xCreatedEventGroup; void PolicyEngine::init() { messagesWaiting = xQueueCreateStatic(PDB_MSG_POOL_SIZE, @@ -56,7 +56,9 @@ void PolicyEngine::init() { } void PolicyEngine::notify(uint32_t notification) { - xEventGroupSetBits(xEventGroupHandle, notification); + if (xEventGroupHandle != NULL) { + xEventGroupSetBits(xEventGroupHandle, notification); + } } void PolicyEngine::pe_task(const void *arg) { @@ -228,20 +230,14 @@ PolicyEngine::policy_engine_state PolicyEngine::pe_sink_eval_cap() { /* New capabilities also means we can't be making a request from the * same PPS APDO */ _last_pps = 8; - /* Get a message object for the request if we don't have one already */ - - /* Remember the last PDO we requested if it was a PPS APDO */ - if (PD_RDO_OBJPOS_GET(&_last_dpm_request) >= _pps_index) { - _last_pps = PD_RDO_OBJPOS_GET(&_last_dpm_request); - /* Otherwise, forget any PPS APDO we had requested */ - } else { - _last_pps = 8; - } /* Ask the DPM what to request */ - pdbs_dpm_evaluate_capability(&tempMessage, &_last_dpm_request); + if (pdbs_dpm_evaluate_capability(&tempMessage, &_last_dpm_request)) { - return PESinkSelectCap; + return PESinkSelectCap; + } + + return PESinkWaitCap; } PolicyEngine::policy_engine_state PolicyEngine::pe_sink_select_cap() { @@ -253,7 +249,7 @@ PolicyEngine::policy_engine_state PolicyEngine::pe_sink_select_cap() { ProtocolTransmit::notify( ProtocolTransmit::Notifications::PDB_EVT_PRLTX_MSG_TX); eventmask_t evt = waitForEvent( - PDB_EVT_PE_TX_DONE | PDB_EVT_PE_TX_ERR | PDB_EVT_PE_RESET, 1000); + PDB_EVT_PE_TX_DONE | PDB_EVT_PE_TX_ERR | PDB_EVT_PE_RESET); /* If we got reset signaling, transition to default */ if (evt & PDB_EVT_PE_RESET || evt == 0) { return PESinkTransitionDefault; @@ -281,10 +277,6 @@ PolicyEngine::policy_engine_state PolicyEngine::pe_sink_select_cap() { /* If the source accepted our request, wait for the new power */ if (PD_MSGTYPE_GET(&tempMessage) == PD_MSGTYPE_ACCEPT && PD_NUMOBJ_GET(&tempMessage) == 0) { - /* Transition to Sink Standby if necessary */ - if (PD_RDO_OBJPOS_GET(&_last_dpm_request) != _last_pps) { - pdbs_dpm_transition_standby(); - } return PESinkTransitionSink; /* If the message was a Soft_Reset, do the soft reset procedure */ diff --git a/workspace/TS100/Core/Drivers/FUSB302/policy_engine_user.cpp b/workspace/TS100/Core/Drivers/FUSB302/policy_engine_user.cpp index 8464e06d..71e14f33 100644 --- a/workspace/TS100/Core/Drivers/FUSB302/policy_engine_user.cpp +++ b/workspace/TS100/Core/Drivers/FUSB302/policy_engine_user.cpp @@ -114,7 +114,7 @@ bool PolicyEngine::pdbs_dpm_evaluate_capability( /* Update requested voltage */ _requested_voltage = 5000; - /* At this point, we have a capability match iff the output is disabled */ + return false; } @@ -194,9 +194,6 @@ bool PolicyEngine::pdbs_dpm_evaluate_typec_current( return true; } -void PolicyEngine::pdbs_dpm_pd_start() { -//PD neg is starting -} void PolicyEngine::pdbs_dpm_transition_default() { /* Cast the dpm_data to the right type */ @@ -207,13 +204,8 @@ void PolicyEngine::pdbs_dpm_transition_default() { pdNegotiationComplete = false; } -void PolicyEngine::pdbs_dpm_transition_min() { -} -void PolicyEngine::pdbs_dpm_transition_standby() { - /* If the voltage is changing, enter Sink Standby */ -} void PolicyEngine::pdbs_dpm_transition_requested() { /* Cast the dpm_data to the right type */ @@ -229,7 +221,7 @@ bool PolicyEngine::messageWaiting() { } bool PolicyEngine::readMessage() { - return xQueueReceive(messagesWaiting, &tempMessage, 1) == pdTRUE; + return xQueueReceive(messagesWaiting, &tempMessage,0) == pdTRUE; } void PolicyEngine::pdbs_dpm_transition_typec() { diff --git a/workspace/TS100/Core/Drivers/FUSB302/protocol_rx.cpp b/workspace/TS100/Core/Drivers/FUSB302/protocol_rx.cpp index 3e7304cf..132cd6a3 100644 --- a/workspace/TS100/Core/Drivers/FUSB302/protocol_rx.cpp +++ b/workspace/TS100/Core/Drivers/FUSB302/protocol_rx.cpp @@ -23,7 +23,9 @@ #include "policy_engine.h" #include "protocol_tx.h" #include "fusb302b.h" -osThreadId ProtocolReceive::TaskHandle; +osThreadId ProtocolReceive::TaskHandle = NULL; +EventGroupHandle_t ProtocolReceive::xEventGroupHandle = NULL; +StaticEventGroup_t ProtocolReceive::xCreatedEventGroup; uint32_t ProtocolReceive::TaskBuffer[ProtocolReceive::TaskStackSize]; osStaticThreadDef_t ProtocolReceive::TaskControlBlock; union pd_msg ProtocolReceive::tempMessage; @@ -138,8 +140,6 @@ ProtocolReceive::protocol_rx_state ProtocolReceive::protocol_rx_store_messageid( return PRLRxWaitPHY; } -EventGroupHandle_t ProtocolReceive::xEventGroupHandle; -StaticEventGroup_t ProtocolReceive::xCreatedEventGroup; void ProtocolReceive::init() { osThreadStaticDef(protRX, thread, PDB_PRIO_PRL, 0, TaskStackSize, TaskBuffer, &TaskControlBlock); @@ -175,10 +175,15 @@ void ProtocolReceive::thread(const void *args) { } void ProtocolReceive::notify(uint32_t notification) { - xEventGroupSetBits(xEventGroupHandle, notification); + if (xEventGroupHandle != NULL) { + xEventGroupSetBits(xEventGroupHandle, notification); + } } uint32_t ProtocolReceive::waitForEvent(uint32_t mask, uint32_t ticksToWait) { - return xEventGroupWaitBits(xEventGroupHandle, mask, mask, - pdFALSE, ticksToWait); + if (xEventGroupHandle != NULL) { + return xEventGroupWaitBits(xEventGroupHandle, mask, mask, + pdFALSE, ticksToWait); + } + return 0; } diff --git a/workspace/TS100/Core/Drivers/FUSB302/protocol_tx.cpp b/workspace/TS100/Core/Drivers/FUSB302/protocol_tx.cpp index 7f63a09f..61bdb101 100644 --- a/workspace/TS100/Core/Drivers/FUSB302/protocol_tx.cpp +++ b/workspace/TS100/Core/Drivers/FUSB302/protocol_tx.cpp @@ -22,16 +22,18 @@ #include "fusb302b.h" #include "fusbpd.h" -osThreadId ProtocolTransmit::TaskHandle; +osThreadId ProtocolTransmit::TaskHandle = NULL; uint32_t ProtocolTransmit::TaskBuffer[ProtocolTransmit::TaskStackSize]; osStaticThreadDef_t ProtocolTransmit::TaskControlBlock; StaticQueue_t ProtocolTransmit::xStaticQueue; bool ProtocolTransmit::messageSending = false; uint8_t ProtocolTransmit::ucQueueStorageArea[PDB_MSG_POOL_SIZE * sizeof(union pd_msg)]; -QueueHandle_t ProtocolTransmit::messagesWaiting; +QueueHandle_t ProtocolTransmit::messagesWaiting = NULL; uint8_t ProtocolTransmit::_tx_messageidcounter; union pd_msg ProtocolTransmit::temp_msg; +EventGroupHandle_t ProtocolTransmit::xEventGroupHandle = NULL; +StaticEventGroup_t ProtocolTransmit::xCreatedEventGroup; /* * PRL_Tx_PHY_Layer_Reset state */ @@ -249,10 +251,10 @@ void ProtocolTransmit::thread(const void *args) { } } -EventGroupHandle_t ProtocolTransmit::xEventGroupHandle; -StaticEventGroup_t ProtocolTransmit::xCreatedEventGroup; void ProtocolTransmit::notify(ProtocolTransmit::Notifications notification) { - xEventGroupSetBits(xEventGroupHandle, (uint32_t) notification); + if (xEventGroupHandle != NULL) { + xEventGroupSetBits(xEventGroupHandle, (uint32_t) notification); + } } void ProtocolTransmit::init() { @@ -266,20 +268,29 @@ void ProtocolTransmit::init() { } void ProtocolTransmit::pushMessage(union pd_msg *msg) { - xQueueSend(messagesWaiting, msg, 100); + if (messagesWaiting) { + xQueueSend(messagesWaiting, msg, 100); + } } bool ProtocolTransmit::messagePending() { - return uxQueueMessagesWaiting(messagesWaiting) > 0; + if (messagesWaiting) { + return uxQueueMessagesWaiting(messagesWaiting) > 0; + } } void ProtocolTransmit::getMessage() { //Loads the pending message into the buffer - xQueueReceive(messagesWaiting, &temp_msg, 1); + if (messagesWaiting) { + xQueueReceive(messagesWaiting, &temp_msg, 1); + } } ProtocolTransmit::Notifications ProtocolTransmit::waitForEvent(uint32_t mask, uint32_t ticksToWait) { - return (Notifications) xEventGroupWaitBits(xEventGroupHandle, mask, mask, - pdFALSE, ticksToWait); + if (xEventGroupHandle) { + return (Notifications) xEventGroupWaitBits(xEventGroupHandle, mask, + mask, + pdFALSE, ticksToWait); + } }