Skip to content

Commit

Permalink
Convert lookup table to _2str function
Browse files Browse the repository at this point in the history 
Summary:
Currently, for some logging, we rely on lookup tables without bounds checking or guarantees about enum values to convert certain values to strings. This could cause MME crashes or incorrect string values in the future.

In this diff, we change this from lookup tables to functions with switch/case statements.

Reviewed By: vikg-fb

Differential Revision: D14690806

fbshipit-source-id: 04518dcda0ddb64d1bb99ebd0e5114027897f232
  • Loading branch information
@sciencemanx @facebook-github-bot
sciencemanx authored and facebook-github-bot committed Apr 3, 2019
1 parent ac3992f commit 5bdee78
Show file tree
Hide file tree
Showing 3 changed files with 41 additions and 37 deletions.
58 changes: 34 additions & 24 deletions lte/gateway/c/oai/tasks/s1ap/s1ap_mme_handlers.c
View file
Expand Up @@ -110,11 +110,6 @@ struct S1ap_E_RABSetupItemBearerSURes_s;
struct S1ap_E_RABSetupItemCtxtSURes_s;
struct S1ap_IE;

static const char *const s1_enb_state_str[] = {"S1AP_INIT",
"S1AP_RESETTING",
"S1AP_READY",
"S1AP_SHUTDOWN"};

static int s1ap_generate_s1_setup_response(
s1ap_state_t *state,
enb_description_t *enb_association);
Expand Down Expand Up @@ -190,13 +185,6 @@ s1ap_message_handler_t message_handlers[][3] = {
{0, 0, 0}, /* UplinkNonUEAssociatedLPPaTransport */
};

const char *s1ap_direction2String[] = {
"", /* Nothing */
"Originating message", /* originating message */
"Successfull outcome", /* successfull outcome */
"UnSuccessfull outcome", /* successfull outcome */
};

int s1ap_mme_handle_message(
s1ap_state_t *state,
const sctp_assoc_id_t assoc_id,
Expand Down Expand Up @@ -229,7 +217,7 @@ int s1ap_mme_handle_message(
"[SCTP %d] No handler for procedureCode %d in %s\n",
assoc_id,
(int) message->procedureCode,
s1ap_direction2String[(int) message->direction]);
s1ap_direction2str(message->direction));
return -2;
}

Expand Down Expand Up @@ -362,7 +350,7 @@ int s1ap_mme_handle_s1_setup_request(
NULL,
0,
"0 S1Setup/%s assoc_id %u stream %u",
s1ap_direction2String[message->direction],
s1ap_direction2str(message->direction),
assoc_id,
stream);

Expand Down Expand Up @@ -411,7 +399,7 @@ int s1ap_mme_handle_s1_setup_request(
OAILOG_WARNING(
LOG_S1AP,
"Ignoring s1setup from eNB in state %s on assoc id %u",
s1_enb_state_str[enb_association->s1_state],
s1_enb_state2str(enb_association->s1_state),
assoc_id);
rc = s1ap_mme_generate_s1_setup_failure(
assoc_id,
Expand Down Expand Up @@ -677,7 +665,7 @@ int s1ap_mme_handle_ue_cap_indication(
0,
"0 UECapabilityInfoIndication/%s enb_ue_s1ap_id " ENB_UE_S1AP_ID_FMT
" mme_ue_s1ap_id " MME_UE_S1AP_ID_FMT " ",
s1ap_direction2String[message->direction],
s1ap_direction2str(message->direction),
ue_cap_p->eNB_UE_S1AP_ID,
ue_cap_p->mme_ue_s1ap_id);

Expand Down Expand Up @@ -782,7 +770,7 @@ int s1ap_mme_handle_initial_context_setup_response(
0,
"0 InitialContextSetup/%s enb_ue_s1ap_id " ENB_UE_S1AP_ID_FMT
" mme_ue_s1ap_id " MME_UE_S1AP_ID_FMT " ",
s1ap_direction2String[message->direction],
s1ap_direction2str(message->direction),
initialContextSetupResponseIEs_p->eNB_UE_S1AP_ID,
initialContextSetupResponseIEs_p->mme_ue_s1ap_id);

Expand Down Expand Up @@ -886,7 +874,7 @@ int s1ap_mme_handle_ue_context_release_request(
0,
"0 UEContextReleaseRequest/%s enb_ue_s1ap_id " ENB_UE_S1AP_ID_FMT
" mme_ue_s1ap_id " MME_UE_S1AP_ID_FMT " ",
s1ap_direction2String[message->direction],
s1ap_direction2str(message->direction),
ueContextReleaseRequest_p->eNB_UE_S1AP_ID,
ueContextReleaseRequest_p->mme_ue_s1ap_id);
// Log the Cause Type and Cause value
Expand Down Expand Up @@ -1376,7 +1364,7 @@ int s1ap_mme_handle_ue_context_release_complete(
0,
"0 UEContextRelease/%s enb_ue_s1ap_id " ENB_UE_S1AP_ID_FMT
" mme_ue_s1ap_id " MME_UE_S1AP_ID_FMT " len %u",
s1ap_direction2String[message->direction],
s1ap_direction2str(message->direction),
ueContextReleaseComplete_p->eNB_UE_S1AP_ID,
ueContextReleaseComplete_p->mme_ue_s1ap_id);

Expand Down Expand Up @@ -1451,7 +1439,7 @@ int s1ap_mme_handle_initial_context_setup_failure(
0,
"0 InitialContextSetup/%s enb_ue_s1ap_id " ENB_UE_S1AP_ID_FMT
" mme_ue_s1ap_id " MME_UE_S1AP_ID_FMT " len %u",
s1ap_direction2String[message->direction],
s1ap_direction2str(message->direction),
initialContextSetupFailureIEs_p->eNB_UE_S1AP_ID,
initialContextSetupFailureIEs_p->mme_ue_s1ap_id);

Expand Down Expand Up @@ -1584,7 +1572,7 @@ int s1ap_mme_handle_ue_context_modification_response(
0,
"0 UEContextModificationResponse/%s enb_ue_s1ap_id " ENB_UE_S1AP_ID_FMT
" mme_ue_s1ap_id " MME_UE_S1AP_ID_FMT " ",
s1ap_direction2String[message->direction],
s1ap_direction2str(message->direction),
ueContextModification_p->eNB_UE_S1AP_ID,
ueContextMofification_p->mme_ue_s1ap_id);

Expand Down Expand Up @@ -1684,7 +1672,7 @@ int s1ap_mme_handle_ue_context_modification_failure(
0,
"0 UEContextModificationFailure/%s enb_ue_s1ap_id " ENB_UE_S1AP_ID_FMT
" mme_ue_s1ap_id " MME_UE_S1AP_ID_FMT " ",
s1ap_direction2String[message->direction],
s1ap_direction2str(message->direction),
ueContextMofification_p->eNB_UE_S1AP_ID,
ueContextMofification_p->mme_ue_s1ap_id);

Expand Down Expand Up @@ -2142,7 +2130,7 @@ int s1ap_handle_new_association(
LOG_S1AP,
"Received new association request on an association that is being %s, "
"ignoring",
s1_enb_state_str[enb_association->s1_state]);
s1_enb_state2str(enb_association->s1_state));
OAILOG_FUNC_RETURN(LOG_S1AP, RETURNerror);
} else {
OAILOG_DEBUG(
Expand Down Expand Up @@ -2246,7 +2234,7 @@ int s1ap_mme_handle_erab_setup_response(
0,
"0 E_RABSetupResponse/%s enb_ue_s1ap_id " ENB_UE_S1AP_ID_FMT
" mme_ue_s1ap_id " MME_UE_S1AP_ID_FMT " len %u",
s1ap_direction2String[message->direction],
s1ap_direction2str(message->direction),
s1ap_E_RABSetupResponseIEs_p->eNB_UE_S1AP_ID,
s1ap_E_RABSetupResponseIEs_p->mme_ue_s1ap_id);

Expand Down Expand Up @@ -2741,3 +2729,25 @@ int s1ap_handle_paging_request(const itti_s1ap_paging_request_t *paging_request)
free_s1ap_paging(paging_message);
OAILOG_FUNC_RETURN(LOG_S1AP, rc);
}

const char *s1_enb_state2str(enum mme_s1_enb_state_s state)
{
switch (state) {
case S1AP_INIT: return "S1AP_INIT";
case S1AP_RESETING: return "S1AP_RESETING";
case S1AP_READY: return "S1AP_READY";
case S1AP_SHUTDOWN: return "S1AP_SHUTDOWN";
default: return "unknown s1ap_enb_state";
}
}

const char *s1ap_direction2str(uint8_t dir)
{
switch (dir) {
case S1AP_PDU_PR_NOTHING: return "<nothing>";
case S1AP_PDU_PR_initiatingMessage: return "originating message";
case S1AP_PDU_PR_successfulOutcome: return "successful outcome";
case S1AP_PDU_PR_unsuccessfulOutcome: return "unsuccessful outcome";
default: return "unknown direction";
}
}
3 changes: 3 additions & 0 deletions lte/gateway/c/oai/tasks/s1ap/s1ap_mme_handlers.h
View file
Expand Up @@ -35,6 +35,9 @@ struct s1ap_message_s;

#define MAX_NUM_PARTIAL_S1_CONN_RESET 256

const char *s1_enb_state2str(enum mme_s1_enb_state_s state);
const char *s1ap_direction2str(uint8_t dir);

/** \brief Handle decoded incoming messages from SCTP
* \param assoc_id SCTP association ID
* \param stream Stream number
Expand Down
17 changes: 4 additions & 13 deletions lte/gateway/c/oai/tasks/s1ap/s1ap_mme_nas_procedures.c
View file
Expand Up @@ -74,15 +74,6 @@
#include "nas/securityDef.h"
#include "s1ap_state.h"

/* Every time a new UE is associated, increment this variable.
But care if it wraps to increment also the mme_ue_s1ap_id_has_wrapped
variable. Limit: UINT32_MAX (in stdint.h).
*/
//static mme_ue_s1ap_id_t mme_ue_s1ap_id = 0;
//static bool mme_ue_s1ap_id_has_wrapped = false;

extern const char *s1ap_direction2String[];

//------------------------------------------------------------------------------
int s1ap_mme_handle_initial_ue_message(
s1ap_state_t *state,
Expand All @@ -109,7 +100,7 @@ int s1ap_mme_handle_initial_ue_message(
NULL,
0,
"0 initialUEMessage/%s assoc_id %u stream %u " ENB_UE_S1AP_ID_FMT " ",
s1ap_direction2String[message->direction],
s1ap_direction2str(message->direction),
assoc_id,
stream,
(enb_ue_s1ap_id_t) initialUEMessage_p->eNB_UE_S1AP_ID);
Expand Down Expand Up @@ -329,7 +320,7 @@ int s1ap_mme_handle_uplink_nas_transport(
0,
"0 uplinkNASTransport/%s mme_ue_s1ap_id " MME_UE_S1AP_ID_FMT
" enb_ue_s1ap_id " ENB_UE_S1AP_ID_FMT " nas len %u",
s1ap_direction2String[message->direction],
s1ap_direction2str(message->direction),
(mme_ue_s1ap_id_t) uplinkNASTransport_p->mme_ue_s1ap_id,
(enb_ue_s1ap_id_t) uplinkNASTransport_p->eNB_UE_S1AP_ID,
uplinkNASTransport_p->nas_pdu.size);
Expand Down Expand Up @@ -357,7 +348,7 @@ int s1ap_mme_handle_uplink_nas_transport(
0,
"0 uplinkNASTransport/%s mme_ue_s1ap_id " MME_UE_S1AP_ID_FMT
" enb_ue_s1ap_id " ENB_UE_S1AP_ID_FMT " nas len %u",
s1ap_direction2String[message->direction],
s1ap_direction2str(message->direction),
(mme_ue_s1ap_id_t) uplinkNASTransport_p->mme_ue_s1ap_id,
(enb_ue_s1ap_id_t) uplinkNASTransport_p->eNB_UE_S1AP_ID,
uplinkNASTransport_p->nas_pdu.size);
Expand Down Expand Up @@ -409,7 +400,7 @@ int s1ap_mme_handle_nas_non_delivery(
0,
"0 NASNonDeliveryIndication/%s mme_ue_s1ap_id " MME_UE_S1AP_ID_FMT
" enb_ue_s1ap_id " ENB_UE_S1AP_ID_FMT " cause %u nas len %u",
s1ap_direction2String[message->direction],
s1ap_direction2str(message->direction),
(mme_ue_s1ap_id_t) nasNonDeliveryIndication_p->mme_ue_s1ap_id,
(enb_ue_s1ap_id_t) nasNonDeliveryIndication_p->eNB_UE_S1AP_ID,
nasNonDeliveryIndication_p->cause,
Expand Down

0 comments on commit 5bdee78

Please sign in to comment.