Realloc stops program on second loop

Question

I was trying to allocate some memory for my data structures (To get data from internet packets), but I faced the problem, that my program just stop, when it goes fro second loop. The first loop goes perfect and all data, inside the structs are correct. Here is the code:

    struct ipOut *ipHeadr = NULL;
    struct ipAddr *addrHeadr = NULL;
    struct hexOut *hexHeadr = NULL;
    struct icmpOut *icmpHeadr = NULL;
    struct igmpOut *igmpHeadr = NULL;
    struct tcpOut *tcpHeadr = NULL;
    struct udpOut *udpHeadr = NULL;

    int main() {

ipHeadr = (struct ipOut*)malloc(sizeof(ipHeadr));
addrHeadr = (struct ipAddr*)malloc(sizeof(addrHeadr));
hexHeadr = (struct hexOut*)malloc(sizeof(hexHeadr));
icmpHeadr = (struct icmpOut*)malloc(sizeof(icmpHeadr));
igmpHeadr = (struct igmpOut*)malloc(sizeof(igmpHeadr));
tcpHeadr = (struct tcpOut*)malloc(sizeof(tcpHeadr));
udpHeadr = (struct udpOut*)malloc(sizeof(udpHeadr));

struct sockaddr saddr;
unsigned char *buff = (unsigned char *)malloc(65536);
int sock = socket(AF_INET , SOCK_RAW , IPPROTO_TCP);
if (sock < 0){
    printf ("Error creating socket");
    return 1;
}
int loop =  1;
while (loop == 1){
    int saddrLength = sizeof saddr;
    rawData = recvfrom(sock, buff, 65536, 0, &saddr, &saddrLength);
    printf("raw data %d\n", rawData);
    if(rawData <0 )
    {
        printf("Failed to get packets\n");
        return 1;
    }
    gettingPacket(buff, rawData);
}
return 0;
    }

    void gettingPacket(unsigned char * buff, int data){
packetNum++;
ipHeadr = (struct ipOut*)realloc(ipHeadr, sizeof(ipHeadr)*packetNum);
addrHeadr = (struct ipAddr*)realloc(addrHeadr,sizeof(addrHeadr)*packetNum);
hexHeadr = (struct hexOut*)realloc(hexHeadr, sizeof(hexHeadr)*packetNum);
struct iphdr *iph = (struct iphdr*)buff;


switch (iph -> protocol)
{
    case 1:
        icmpNum++;
        icmpHeadr = (struct icmpOut*)realloc(icmpHeadr, sizeof(icmpHeadr)*icmpNum);
        icmpOutput(buff, data);
        hexDataOut(buff, data);
        break;
    case 2:
        igmpNum++;
        igmpHeadr = (struct igmpOut*)realloc(igmpHeadr,sizeof(igmpHeadr)*igmpNum);
        igmpOutput(buff, data);
        hexDataOut(buff, data);
        break;
    case 6:
        tcpNum++;
        tcpHeadr = (struct tcpOut*)realloc(tcpHeadr,sizeof(tcpHeadr)*tcpNum);
        tcpPacketOutput(buff, data);
        hexDataOut(buff, data);
        break;
    case 17:
        udpNum++;
        udpHeadr = (struct udpOut*)realloc(udpHeadr, sizeof(udpHeadr)*udpNum);
        udpPacketOutput(buff, data);
        hexDataOut(buff, data);
        break;

}
    }

Debugging tells that code stops on this line:

    ipHeadr = (struct ipOut*)realloc(ipHeadr, sizeof(ipHeadr)*packetNum);

I think there is a memory leak somewhere, but I cant find it.

Hint `Type *pointer = malloc(sizeof(*pointer));`, `sizeof(pointer)` is pointer size. `sizeof(Type)` or `sizeof(*pointer)` is Type size. — BLUEPIXY, Jun 17 '17 at 13:52
Use [Valgrind](http://valgrind.org/) to find the leak. Version 3.13.0 was released yesterday. — Jonathan Leffler, Jun 17 '17 at 13:59
[don't cast the result of `malloc` in C](http://stackoverflow.com/q/605845/995714) — phuclv, Jun 17 '17 at 14:04
@Olaf That a contentious point, besides, some compilers require casting. — Michaël Roy, Jun 17 '17 at 17:12
@MichaëlRoy: It is consensus about all experienced programmers (not only C) not to use unnecessary casts. No C compiler requires a cast, read the standard! Just in case you mean a C++ compiler: tht's a very different language, C++ is **not** C compatible! — too honest for this site, Jun 18 '17 at 00:15
Yes it is contentious, and it will be as long as students learn from their college professors that they should, and that most senior programmers will continue to cast the result of malloc. I've been casting the return value of malloc since 1990 and that has never caused a single bug in my code. — Michaël Roy, Jun 18 '17 at 00:27

Michaël Roy · Accepted Answer · 2017-06-18T01:04:27.507

You are not filling your buffers with any data, so they are a bit useless.

Also, in your call to realloc:

icmpHeadr = (struct icmpOut*)realloc(icmpHeadr, sizeof(icmpHeadr) * icmpNum);
/* the size given above is wrong, sizeof(icmpHeadr) is the size of a pointer. */

You should keep realloc and the copy of data within the buffer together. Such as:

void main(..)
{
void* tmp;

/*  no need to initialize your arrays here
    a NULL value would indicate an empty array */

/*...*/

  case 1:
    /* icmpHeadr can be NULL on first call to realloc that's not a problem
       but realloc could fail if you run out of memory...
    */
    tmp = realloc(icmpHeadr, sizeof(struct icmpOut)*(icmpNum + 1));
    if (tmp)
    {
       icmpHeadr = (struct icmpOut*)tmp;
       memcpy(&icmpHeadr[icmpNum], buff, sizeof(struct icmpOut));
       ++icmpNum;
       icmpOutput(buff, data);
       hexDataOut(buff, data);
    }
    else 
    {
      /* you've ran out of memory 
         You could keep counting icmp messages
         but that would mess your array size, in case memory 
         becomes available later.

         ...or you could simply exit your app.  Avoiding running out of
         memory would necessitate a major rewrite of your app, anyway.
      */
    }
    break;

Realloc stops program on second loop

1 Answers1