C allocation memory

Question

I'm a beginner on C, and I'm with a bug, the code bellow in some point stops the application during the runtime.

int l = 0;
//block 1
char *primaryIPaddr;
unsigned char *primaryIPaddrSize;
primaryIPaddrSize = *(unsigned char *) &msg[l++];
AddToLog(logInfo, "PrimarySize: %u\n", primaryIPaddrSize);
if(primaryIPaddrSize != NULL) {
  memcpy ( primaryIPaddr, &msg[l], primaryIPaddrSize );
  AddToLog(logInfo, "PrimaryAddr: %s\n", primaryIPaddr);
  l += primaryIPaddrSize;
}
//block 2
char *secondaryIPaddr;
unsigned char *secondaryIPaddrSize;
secondaryIPaddrSize = *(unsigned char *) &msg[l++];
AddToLog(logInfo, "secondarySize: %u\n", secondaryIPaddrSize);
if(secondaryIPaddrSize != NULL) {
  memcpy ( secondaryIPaddr, &msg[l], secondaryIPaddrSize );
  AddToLog(logInfo, "secondaryAddr: %s\n", secondaryIPaddr);
  l += secondaryIPaddrSize;
}
//block 3
unsigned int *primaryPort;
unsigned int *secondaryPort;
primaryPort = *(unsigned int *) &msg[l++];
AddToLog(logInfo, "primaryPort: %u\n", primaryPort);
secondaryPort = *(unsigned int *) &msg[l++];
AddToLog(logInfo, "secondaryPort: %u\n", secondaryPort);`

The msg variable it's a char filled by a UDP packet, I don't know what is wrong with my code, but when the process get to the block 2 it just stops, sounds like a memory problem, sinceI remove the block 1 and advance the l variable to the exactly point of the block 2, the block 2 process and dump correctly, just like the block 1 before the block 2 stops the process. So the code itself it's ok, seems like I'm not allocating memory properly, I tried to use malloc, but didn't do any effect so, I don't know what I'm not doing right, someone know the answer of what could be happening here?

The UDP packet comes this way:

UINT8 primaryIPaddrSize;
UINT8* primaryIPaddr; primaryIPaddrSize bytes
UINT8 secondaryIPaddrSize;
UINT8* secondaryIPaddr; secondaryIPaddrSize bytes
UINT16 primaryIPport;
UINT16 secondaryIPport`

The secondaryIp it's optional, so if the secondaryIPaddrSize is null, than I must jump secondaryIPaddr.`

Answer 1

Your 'block 1' has a couple of problems. You need to turn on your compiler warnings, or pay attention to those it is giving you.

char *primaryIPaddr;
unsigned char *primaryIPaddrSize;
primaryIPaddrSize = *(unsigned char *) &msg[l++];
AddToLog(logInfo, "PrimarySize: %u\n", primaryIPaddrSize);
if (primaryIPaddrSize != NULL) {
    memcpy(primaryIPaddr, &msg[l], primaryIPaddrSize );
    AddToLog(logInfo, "PrimaryAddr: %s\n", primaryIPaddr);
    l += primaryIPaddrSize;
}

Your 'primaryIPaddrSize' is a char * (address), yet in the memcpy(), you use it as a length. The assignment to primaryIPaddrSize is odd; you should be getting a warning about assigning a char to a pointer. Given the assignment, the memcpy() length is 'OK', but it is all weird. You should be writing:

unsigned char primaryIPaddrSize = msg[l++];

The other problem (that I'm going to diagnose — there are probably others) is that your memcpy() is copying to indeterminate memory. You've not initialized primaryIPaddr, so it could point anywhere in memory.

You must always make sure you know where the data is going to be stored. Arguably, you need:

char *primaryIPaddr = malloc(primaryIPaddrSize);

followed by a memory check.

char *primaryIPaddr;
unsigned char primaryIPaddrSize = msg[l++];
AddToLog(logInfo, "PrimarySize: %u\n", primaryIPaddrSize);
char *primaryIPaddr = NULL;
if (primaryIPaddrSize != 0)
{
    primaryIPaddr = malloc(primaryIPaddrSize);
    if (primaryIPaddr == 0)
        ...handle out of memory...
    memcpy(primaryIPaddr, &msg[l], primaryIPaddrSize );
    AddToLog(logInfo, "PrimaryAddr: %s\n", primaryIPaddr);
    l += primaryIPaddrSize;
}

The problems in Block 2 are pretty much symmetric with those in Block 1, but you've already wrecked the system by copying data to an unknown location. The code in Block 3 doesn't need the pointers, again. Also, port numbers are usually 2 bytes (0..65535), but your code only appears to handle 1 byte port numbers (so you won't be supporting HTTPS, amongst other services).