Processing text messages from a GSM module

I have a Tiva TM4C ARM processor connected to a GSM modem (SIM900). When the GSM modem receives a new text message it sends a UART string to the MCU, where an interrupt routine increments a new message counter.

Then my program takes over to parse the message. Below is the code. This code currently functions correctly. However, I believe that there may be a better way to do this than using pointers - especially since I don't have a great understanding of how pointers function, and much of those code was adapted from stuff I found online.

I would love insight on ways this code could be improved.

```
bool testDelete = 0; // Delete messages during processing
int msgCount = 0; // Hold the int value w/count of new messages
// (Set by UART interrupt handler)

// Data from most recent incoming message stored here
char responseLine[10][75]; // Use to store UART inputs
char *msgContent = NULL; // Message content holder
char *msgSender = NULL; // Message sender
char *msgDate = NULL; // Message date
char *msgTime = NULL; // Message time

//***
//
// PROCESS SMS FOR ENVELOPE AND CONTENT
//
//***
void
GSMprocessMessage( int msgNum )
{
bool msgPresent[4] = {0000}; // Flag to ignore deleted messages
bool msgVerify = false; // Flag for message error checking
char msgErrorCheck[4][225]; // Holder for message error checking
int lineCount; // Hold the number of lines
int oLoop; // Counter for outside error checking loop
int iLoop; // Counter for inside error checking loop

// Start message retrieval/parsing/error checking (runs simultaneously to
// reduce calls to the SIM module).
for ( oLoop=0; oLoop

Avoid overlapping strings with strncpy()

I can't speak to the correctness of your algorithm because I don't know why you need to read the same message up to 4 times. But I did spot something non-standard in your code. You are trying to strip a prefix from a string like this:

strncpy(msgSender,msgSender+2,11);
        msgSender[11] = '\0';

Here you are trying to erase the first two characters of msgSender. However, strcpy() and strncpy() technically do not allow the source and destination strings to be overlapping. From the Linux man page for strcpy():


The strcpy() function copies the string pointed to by src, including
the terminating null byte ('\0'), to the buffer pointed to by dest.
The strings may not overlap, and the destination string dest must be
large enough to receive the copy.

No need to copy

Actually, you shouldn't be using strncpy() in the first place. To "remove" the prefix characters, you can simply move your pointer forward. For example, the above code could be replaced with:

msgSender += 2;          // Skip the "+1" prefix from the phone number.

The part to terminate the string with a null character shouldn't be necessary because strtok() already did that. You would only need to terminate with a null character if you think that there may be a suffix you want to cut off:

msgSender[10] = '\0';    // Limit the phone # to 10 digits.

Multiple calls to strcat() not efficient

It's not efficient to keep calling strcat() on the same string because strcat() needs to measure the length of the current string before appending the new part. If you append \$n\$ strings using strcat(), it is an \$O(n)\$ operation. So this code:

strcpy(msgErrorCheck[oLoop], msgSender);
        strcat(msgErrorCheck[oLoop], msgDate);
        strcat(msgErrorCheck[oLoop], msgTime);
        strcat(msgErrorCheck[oLoop], msgContent);

would be better off like this:

sprintf(msgErrorCheck[oLoop], "%s%s%s%s", msgSender, msgDate,
                msgTime, msgContent);

In this case, since the strings are so short and there are only 4 of them, this change will not even be noticeable.

StackExchange Code Review Q#113196, answer score: 2