call: Refactor code, check for NULL pointers and prevent memory leaks
authorPaul Kocialkowski <contact@paulk.fr>
Sun, 31 Mar 2013 13:52:24 +0000 (15:52 +0200)
committerPaul Kocialkowski <contact@paulk.fr>
Sun, 31 Mar 2013 13:52:24 +0000 (15:52 +0200)
Signed-off-by: Paul Kocialkowski <contact@paulk.fr>
call.c
samsung-ril.h

diff --git a/call.c b/call.c
index a784b3e..4258bf1 100644 (file)
--- a/call.c
+++ b/call.c
 
 #include "samsung-ril.h"
 
-/*
- * Format conversion utils
- */
+unsigned char ril2ipc_call_identity(int clir)
+{
+       switch (clir) {
+               case 0:
+                       return IPC_CALL_IDENTITY_DEFAULT;
+               case 1:
+                       return IPC_CALL_IDENTITY_SHOW;
+               case 2:
+                       return IPC_CALL_IDENTITY_HIDE;
+               default:
+                       LOGE("Unknown call identity: 0x%x", clir);
+                       return IPC_CALL_IDENTITY_DEFAULT;
+       }
+}
 
 unsigned char ipc2ril_call_list_entry_state(unsigned char call_state)
 {
-       switch(call_state) {
+       switch (call_state) {
                case IPC_CALL_LIST_ENTRY_STATE_ACTIVE:
                        return RIL_CALL_ACTIVE;
                case IPC_CALL_LIST_ENTRY_STATE_HOLDING:
@@ -44,223 +55,184 @@ unsigned char ipc2ril_call_list_entry_state(unsigned char call_state)
                case IPC_CALL_LIST_ENTRY_STATE_WAITING:
                        return RIL_CALL_WAITING;
                default:
-                       LOGE("Unknown IPC_CALL_LIST_ENTRY_STATE!");
+                       LOGE("Unknown call list entry state: 0x%x", call_state);
                        return -1;
        }
 }
 
 RIL_LastCallFailCause ipc2ril_call_fail_cause(unsigned char end_cause)
 {
-       switch(end_cause) {
+       switch (end_cause) {
                case IPC_CALL_END_CAUSE_NORMAL:
                case IPC_CALL_END_CAUSE_REJECTED:
                        return CALL_FAIL_NORMAL;
                case IPC_CALL_END_CAUSE_UNSPECIFIED:
                default:
+                       LOGE("Unknown call fail cause: 0x%x", end_cause);
                        return CALL_FAIL_ERROR_UNSPECIFIED;
        }
 }
 
-/*
- * In: RIL_UNSOL_CALL_RING
- *   Ring indication for an incoming call (eg, RING or CRING event).
- */
 void ipc_call_incoming(struct ipc_message_info *info)
 {
        ril_request_unsolicited(RIL_UNSOL_CALL_RING, NULL, 0);
 
-       /* FIXME: Do we really need to send this? */
        ril_request_unsolicited(RIL_UNSOL_RESPONSE_CALL_STATE_CHANGED, NULL, 0);
 }
 
-/*
- * In: IPC_CALL_STATUS
- *   Indicates that a call's status has changed
- *
- * RIL_UNSOL_RESPONSE_CALL_STATE_CHANGED
- *   Indicate when call state has changed
- */
 void ipc_call_status(struct ipc_message_info *info)
 {
-       struct ipc_call_status *call_status =
-               (struct ipc_call_status *) info->data;
+       struct ipc_call_status *call_status;
 
-       memcpy(&(ril_data.state.call_status), call_status, sizeof(struct ipc_call_status));
+       if (info == NULL || info->data == NULL || info->length < sizeof(struct ipc_call_status))
+               return;
+
+       call_status = (struct ipc_call_status *) info->data;
 
-       LOGD("Updating Call Status data");
+       LOGD("Updating call status data");
+       memcpy(&(ril_data.state.call_status), call_status, sizeof(struct ipc_call_status));
 
        ril_request_unsolicited(RIL_UNSOL_RESPONSE_CALL_STATE_CHANGED, NULL, 0);
 }
 
-/*
- * In: RIL_REQUEST_DIAL
- *   Initiate voice call
- *
- * Out: IPC_CALL_OUTGOING
- */
-void ril_request_dial(RIL_Token t, void *data, size_t datalen)
+void ril_request_dial(RIL_Token t, void *data, size_t length)
 {
-       const RIL_Dial *dial = (const RIL_Dial*)data;
+       RIL_Dial *dial;
        struct ipc_call_outgoing call;
        int clir;
 
+       if (data == NULL || length < sizeof(RIL_Dial))
+               goto error;
+
+       dial = (RIL_Dial *) data;
+
        if (strlen(dial->address) > sizeof(call.number)) {
                printf("Outgoing call number too long\n");
-               return;
-       }
-
-       /* FIXME: separate method? */
-       switch(dial->clir) {
-               case 0:
-                       clir = IPC_CALL_IDENTITY_DEFAULT;
-                       break;
-               case 1:
-                       clir = IPC_CALL_IDENTITY_SHOW;
-                       break;
-               case 2:
-                       clir = IPC_CALL_IDENTITY_HIDE;
-                       break;
-               default:
-                       clir = IPC_CALL_IDENTITY_DEFAULT;
-                       break;
+               goto error;
        }
 
-       memset(&call, 0x00, sizeof(call));
-
+       memset(&call, 0, sizeof(call));
        call.type = IPC_CALL_TYPE_VOICE;
-       call.identity = clir;
+       call.identity = ril2ipc_call_identity(dial->clir);
        call.prefix = dial->address[0] == '+' ? IPC_CALL_PREFIX_INTL : IPC_CALL_PREFIX_NONE;
-
        call.length = strlen(dial->address);
        memcpy(call.number, dial->address, strlen(dial->address));
 
+       ipc_gen_phone_res_expect_to_complete(ril_request_get_id(t), IPC_CALL_OUTGOING);
+
        ipc_fmt_send(IPC_CALL_OUTGOING, IPC_TYPE_EXEC, (unsigned char *) &call, sizeof(call), ril_request_get_id(t));
 
-       /* FIXME: This should actually be sent based on the response from baseband */
-       ril_request_complete(t, RIL_E_SUCCESS, NULL, 0);
+       return;
+
+error:
+       ril_request_complete(t, RIL_E_GENERIC_FAILURE, NULL, 0);
 }
 
-/*
- * In: RIL_REQUEST_GET_CURRENT_CALLS
- *   Requests current call list
- *
- * Out: IPC_CALL_LIST GET
- *   Requests a list of active calls
- */
 void ril_request_get_current_calls(RIL_Token t)
 {
        ipc_fmt_send_get(IPC_CALL_LIST, ril_request_get_id(t));
 }
 
-/*
- * In: IPC_CALL_LIST GET
- *   Provides a list of active calls
- *
- * Out: RIL_REQUEST_GET_CURRENT_CALLS
- *   Requests current call list
- */
 void ipc_call_list(struct ipc_message_info *info)
 {
        struct ipc_call_list_entry *entry;
-       unsigned char num_entries;
-       char *number, *number_ril;
+       unsigned char count;
+       char *number;
+       RIL_Call **current_calls = NULL;
        int i;
 
-       num_entries = *((unsigned char *) info->data);
+       if (info == NULL || info->data == NULL || info->length < sizeof(unsigned char))
+               goto error;
 
-       if (num_entries == 0) {
-               // Don't bother with mem alloc
+       if (info->type != IPC_TYPE_RESP)
+               return;
 
+       count = *((unsigned char *) info->data);
+       if (count == 0) {
                ril_request_complete(ril_request_get_token(info->aseq), RIL_E_SUCCESS, NULL, 0);
                return;
        }
 
-       entry = (struct ipc_call_list_entry *) ((char *) info->data + 1);
+       current_calls = (RIL_Call **) calloc(1, count * sizeof(RIL_Call *));
+       entry = (struct ipc_call_list_entry *) ((char *) info->data + sizeof(unsigned char));
 
-       RIL_Call **calls = (RIL_Call **) malloc(num_entries * sizeof(RIL_Call *));
+       for (i=0 ; i < count ; i++) {
+               if (((int) entry - (int) info->data) >= (int) info->length)
+                       goto error;
 
-       for (i = 0; i < num_entries; i++) {
-               RIL_Call *call = (RIL_Call *) malloc(sizeof(RIL_Call));
+               number = ((char *) entry) + sizeof(struct ipc_call_list_entry);
 
-               /* Number is located after call list entry */
-               number = ((char *) entry) + sizeof(*entry);
+               current_calls[i] = (RIL_Call *) calloc(1, sizeof(RIL_Call));
 
-               number_ril = (char *) malloc(entry->number_len + 1);
-               memset(number_ril, 0, (entry->number_len + 1));
-               memcpy(number_ril, number, entry->number_len);
+               current_calls[i]->state = ipc2ril_call_list_entry_state(entry->state);
+               current_calls[i]->index = entry->idx;
+               current_calls[i]->toa = (entry->number_len > 0 && number[0] == '+') ? 145 : 129;
+               current_calls[i]->isMpty = entry->mpty;
+               current_calls[i]->isMT = (entry->term == IPC_CALL_TERM_MT);
+               current_calls[i]->als = 0;
+               current_calls[i]->isVoice  = (entry->type == IPC_CALL_TYPE_VOICE);
+               current_calls[i]->isVoicePrivacy = 0;
+               current_calls[i]->number = strdup(number);
+               current_calls[i]->numberPresentation = (entry->number_len > 0) ? 0 : 2;
+               current_calls[i]->name = NULL;
+               current_calls[i]->namePresentation = 2;
+               current_calls[i]->uusInfo = NULL;
 
-               call->state = ipc2ril_call_list_entry_state(entry->state);
-               call->index = (entry->idx+1);
-               call->toa = (entry->number_len > 0 && number[0] == '+') ? 145 : 129;
-               call->isMpty = entry->mpty;
-               call->isMT = (entry->term == IPC_CALL_TERM_MT);
-               call->als = 0;
-               call->isVoice  = (entry->type == IPC_CALL_TYPE_VOICE);
-               call->isVoicePrivacy = 0;
-               call->number = number_ril;
-               call->numberPresentation = (entry->number_len > 0) ? 0 : 2;
-               call->name = NULL;
-               call->namePresentation = 2;
-               call->uusInfo = NULL;
+               entry = (struct ipc_call_list_entry *) (number + entry->number_len);
+       }
 
-               calls[i] = call;
+       ril_request_complete(ril_request_get_token(info->aseq), RIL_E_SUCCESS, current_calls, (count * sizeof(RIL_Call *)));
 
-               /* Next entry after current number */
-               entry = (struct ipc_call_list_entry *) (number + entry->number_len);
+       for (i=0 ; i < count ; i++) {
+               if (current_calls[i]->number != NULL)
+                       free(current_calls[i]->number);
+
+               free(current_calls[i]);
        }
 
-       ril_request_complete(ril_request_get_token(info->aseq), RIL_E_SUCCESS, calls, (num_entries * sizeof(RIL_Call *)));
+       free(current_calls);
+
+       return;
 
-       for (i = 0; i < num_entries; i++) {
-               free(calls[i]);
+error:
+       if (current_calls != NULL) {
+               for (i=0 ; i < count ; i++) {
+                       if (current_calls[i]->number != NULL)
+                               free(current_calls[i]->number);
+
+                       free(current_calls[i]);
+               }
+
+               free(current_calls);
        }
 
-       free(calls);
+       if (info != NULL && info->type == IPC_TYPE_RESP)
+               ril_request_complete(ril_request_get_token(info->aseq), RIL_E_GENERIC_FAILURE, NULL, 0);
 }
 
-/*
- * In: RIL_REQUEST_HANGUP
- *   Hang up a specific line (like AT+CHLD=1x)
- *
- * Out: IPC_CALL_RELEASE EXEC
- */
 void ril_request_hangup(RIL_Token t)
 {
-       ipc_fmt_send_exec(IPC_CALL_RELEASE, ril_request_get_id(t));
+       ipc_gen_phone_res_expect_to_complete(ril_request_get_id(t), IPC_CALL_RELEASE);
 
-       /* FIXME: This should actually be sent based on the response from baseband */
-       ril_request_complete(t, RIL_E_SUCCESS, NULL, 0);
+       ipc_fmt_send_exec(IPC_CALL_RELEASE, ril_request_get_id(t));
 
-       /* FIXME: Do we really need to send this? */
        ril_request_unsolicited(RIL_UNSOL_RESPONSE_CALL_STATE_CHANGED, NULL, 0);
 }
 
-/*
- * In: RIL_REQUEST_ANSWER
- *   Answer incoming call
- *
- * Out: IPC_CALL_ANSWER
- */
+
 void ril_request_answer(RIL_Token t)
 {
-       ipc_fmt_send_exec(IPC_CALL_ANSWER, ril_request_get_id(t));
+       ipc_gen_phone_res_expect_to_complete(ril_request_get_id(t), IPC_CALL_ANSWER);
 
-       /* FIXME: This should actually be sent based on the response from baseband */
-       ril_request_complete(t, RIL_E_SUCCESS, NULL, 0);
+       ipc_fmt_send_exec(IPC_CALL_ANSWER, ril_request_get_id(t));
 
-       /* FIXME: Do we really need to send this? */
        ril_request_unsolicited(RIL_UNSOL_RESPONSE_CALL_STATE_CHANGED, NULL, 0);
 }
 
-/*
- * In: RIL_REQUEST_LAST_CALL_FAIL_CAUSE
- *   Reason why last call was terminated
- */
 void ril_request_last_call_fail_cause(RIL_Token t)
 {
        RIL_LastCallFailCause fail_cause;
-       struct ipc_call_status *call_status =
-               &(ril_data.state.call_status);
+       struct ipc_call_status *call_status = &(ril_data.state.call_status);
 
        fail_cause = ipc2ril_call_fail_cause(call_status->end_cause);
 
@@ -270,26 +242,25 @@ void ril_request_last_call_fail_cause(RIL_Token t)
        ril_request_complete(t, RIL_E_SUCCESS, &fail_cause, sizeof(RIL_LastCallFailCause));
 }
 
-/*
- * In: IPC_CALL_BURST_DTMF
- *   Send DTMF burst. RILJ only sends 1 DTMF tone to send at a time.
- *
- * Out: IPC_CALL_BURST_DTMF
- *   It should be possible to send multiple DTMF tones at once in this message.
- *   First byte should be DTMF tones count.
- */
 void ril_request_dtmf(RIL_Token t, void *data, int length)
 {
        struct ipc_call_cont_dtmf cont_dtmf;
+       unsigned char tone;
+       unsigned char count;
 
        unsigned char *burst;
-       int burst_len;
+       int burst_length;
 
-       unsigned char dtmf_count = 1;
        int i;
 
+       if (data == NULL || length < (int) sizeof(unsigned char))
+               goto error;
+
+       tone = *((unsigned char *) data);
+       count = 1;
+
        if (ril_data.state.dtmf_tone != 0) {
-               LOGD("Another tone wasn't stopped, stopping that one before anything");
+               LOGD("Another tone wasn't stopped, stopping it before anything");
 
                cont_dtmf.state = IPC_CALL_DTMF_STATE_STOP;
                cont_dtmf.tone = 0;
@@ -299,76 +270,101 @@ void ril_request_dtmf(RIL_Token t, void *data, int length)
                usleep(300);
        }
 
-       burst_len = sizeof(struct ipc_call_cont_dtmf) * dtmf_count + 1;
-       burst = malloc(burst_len);
-       memset(burst, 0, burst_len);
+       burst_length = sizeof(struct ipc_call_cont_dtmf) * count + 1;
+       burst = calloc(1, burst_length);
 
-       burst[0] = dtmf_count;
-
-       for (i=0 ; i < dtmf_count ; i++) {
-               // Apparently, it's possible to set multiple DTMF tones on this message
+       burst[0] = count;
 
+       // Apparently, it's possible to set multiple DTMF tones on this message
+       for (i=0 ; i < count ; i++) {
                cont_dtmf.state = IPC_CALL_DTMF_STATE_START;
-               cont_dtmf.tone = ((char *) data)[0];
+               cont_dtmf.tone = tone;
 
                memcpy(burst + 1 + sizeof(struct ipc_call_cont_dtmf) * i, &cont_dtmf, sizeof(cont_dtmf));
        }
 
        ipc_gen_phone_res_expect_to_abort(ril_request_get_id(t), IPC_CALL_BURST_DTMF);
 
-       ipc_fmt_send(IPC_CALL_BURST_DTMF, IPC_TYPE_EXEC, (void *) burst, burst_len, ril_request_get_id(t));
+       ipc_fmt_send(IPC_CALL_BURST_DTMF, IPC_TYPE_EXEC, burst, burst_length, ril_request_get_id(t));
 
        free(burst);
+
+       return;
+
+error:
+       ril_request_complete(t, RIL_E_GENERIC_FAILURE, NULL, 0);
 }
 
 void ipc_call_burst_dtmf(struct ipc_message_info *info)
 {
-       unsigned char ret = *((unsigned char *) info->data);
+       unsigned char code;
 
-       // This apparently should return 1, or perhaps that is the DTMF tones count
-       if (ret == 0) {
-               LOGD("Apparently, something went wrong with DTMF burst");
+       if (info == NULL || info->data == NULL || info->length < sizeof(unsigned char))
+               goto error;
 
-               ril_request_complete(ril_request_get_token(info->aseq), RIL_E_GENERIC_FAILURE, NULL, 0);
+       code = *((unsigned char *) info->data);
+
+       // This apparently should return 1, or perhaps that is the DTMF tones count
+       if (code == 0) {
+               LOGD("Apparently, something went wrong with DTMF burst (code=0x%x)", code);
+               goto error;
        }
 
        ril_request_complete(ril_request_get_token(info->aseq), RIL_E_SUCCESS, NULL, 0);
+
+       return;
+
+error:
+       if (info != NULL)
+               ril_request_complete(ril_request_get_token(info->aseq), RIL_E_GENERIC_FAILURE, NULL, 0);
 }
 
 void ril_request_dtmf_start(RIL_Token t, void *data, int length)
 {
        struct ipc_call_cont_dtmf cont_dtmf;
+       unsigned char tone;
+
+       if (data == NULL || length < (int) sizeof(unsigned char))
+               goto error;
+
+       tone = *((unsigned char *) data);
 
        if (ril_data.state.dtmf_tone != 0) {
-               LOGD("Another tone wasn't stopped, stopping that one before anything");
+               LOGD("Another tone wasn't stopped, stopping it before anything");
 
                cont_dtmf.state = IPC_CALL_DTMF_STATE_STOP;
                cont_dtmf.tone = 0;
 
-               ipc_fmt_send(IPC_CALL_CONT_DTMF, IPC_TYPE_SET, (void *) &cont_dtmf, sizeof(cont_dtmf), ril_request_get_id(t));
+               ipc_fmt_send(IPC_CALL_CONT_DTMF, IPC_TYPE_SET, (unsigned char *) &cont_dtmf, sizeof(cont_dtmf), ril_request_get_id(t));
 
                usleep(300);
        }
 
-       cont_dtmf.state = IPC_CALL_DTMF_STATE_START;
-       cont_dtmf.tone = ((unsigned char *)data)[0];
-
        ril_data.state.dtmf_tone = cont_dtmf.tone;
 
+       cont_dtmf.state = IPC_CALL_DTMF_STATE_START;
+       cont_dtmf.tone = tone;
+
        ipc_gen_phone_res_expect_to_complete(ril_request_get_id(t), IPC_CALL_CONT_DTMF);
 
-       ipc_fmt_send(IPC_CALL_CONT_DTMF, IPC_TYPE_SET, (void *) &cont_dtmf, sizeof(cont_dtmf), ril_request_get_id(t));
+       ipc_fmt_send(IPC_CALL_CONT_DTMF, IPC_TYPE_SET, (unsigned char *) &cont_dtmf, sizeof(cont_dtmf), ril_request_get_id(t));
+
+       return;
+
+error:
+       ril_request_complete(t, RIL_E_GENERIC_FAILURE, NULL, 0);
 }
 
 void ril_request_dtmf_stop(RIL_Token t)
 {
        struct ipc_call_cont_dtmf cont_dtmf;
-       cont_dtmf.state = IPC_CALL_DTMF_STATE_STOP;
-       cont_dtmf.tone = 0;
 
        ril_data.state.dtmf_tone = 0;
 
+       cont_dtmf.state = IPC_CALL_DTMF_STATE_STOP;
+       cont_dtmf.tone = 0;
+
        ipc_gen_phone_res_expect_to_complete(ril_request_get_id(t), IPC_CALL_CONT_DTMF);
 
-       ipc_fmt_send(IPC_CALL_CONT_DTMF, IPC_TYPE_SET, (void *) &cont_dtmf, sizeof(cont_dtmf), ril_request_get_id(t));
+       ipc_fmt_send(IPC_CALL_CONT_DTMF, IPC_TYPE_SET, (unsigned char *) &cont_dtmf, sizeof(cont_dtmf), ril_request_get_id(t));
 }
index 0b14972..d8aae14 100644 (file)
@@ -383,7 +383,7 @@ void ipc_sms_device_ready(struct ipc_message_info *info);
 
 void ipc_call_incoming(struct ipc_message_info *info);
 void ipc_call_status(struct ipc_message_info *info);
-void ril_request_dial(RIL_Token t, void *data, size_t datalen);
+void ril_request_dial(RIL_Token t, void *data, size_t length);
 void ril_request_get_current_calls(RIL_Token t);
 void ipc_call_list(struct ipc_message_info *info);
 void ril_request_hangup(RIL_Token t);