when should I use memcpy()?

Hi,
I am newer of learning c programing, I want to know when should I use memcpy() in my program?

Code:

int main() {
    char *a = NULL;
    char *b = NULL;

    b = malloc (sizeof(char) * 5);
    a = b; /*red code */
    memcpy (a, b, sizeof(char)*5); /* blue code */
    
    return (0);
}

Which one is better between red code and blue code?
thx

What are you trying to do?

There is no "one true answer". It depends what you want to do. The red version makes a and b point to the same exact piece of memory. When you free(b), which you really should do, both a and b will point to the same, now invalid (you freed it) memory. The blue code is just wrong here, since you never allocate memory for a, so you can't actually copy anything in there. In fact, a points to NULL, which basically means "not a memory address". You would have to malloc at least 5 bytes for a too, if you want to use memcpy. Note, you don't need sizeof(char) in your malloc statements, since it is guaranteed by the standard to always be 1.

Back to the "it depends" bit. If you want two pointers pointing to the same piece of memory, you would use the red code. You might do this if you wanted to iterate through the bytes of b with the pointer a, such as searching for a particular byte. If you want a copy of the thing pointed to by b, you would use the blue code (of course malloc'ing memory for a first). You might do this when you want to change some data but retain a copy of the original, such as sorting the bytes and retaining a copy of the original for comparison. Those might not be the best examples (it's late and I'm tired), and there are many more use cases for both the red and the blue method, you really have to treat it on a case-by-case basis.

Note, often, allocating arrays of chars means you're using strings, which must be null terminated to work correctly. If you are using strings, you would of course want to use the string functions, like strlen, strcpy, etc, which specifically handle null terminated strings.

Apart from anduril's comments, surely it's quite simple.

When you want to COPY MEMORY, you use memcpy. What you don't want to copy memory, you don't. That might be because you want a different size of memory, use it for different types, start with it blank rather than a copy of something else, don't just want two things to point at the same memory but have separate copies, etc.

There's a reason functions are named like they are. memcpy copies some existing memory. malloc allocates some new memory. memset sets some area of memory to a preset value. calloc allocates and clears memory (so like malloc + memset put together in one command).

thx laserlight, anduril462, ledow

I am programing with libpcap to catch packets from my net interface.

Code:

for (status=0, pcount=0, in=0; (status>=0)&&(pcount<PCOUNT); pcount++) {
        status = pcap_next_ex (phandle, &pkt_header, &pkt_data);
        if (status == 0) {
            /* TIMEOUT */
            continue;
        }
        if (status == -1) {
            printf ("Error reading the packets: %s", pcap_geterr(phandle));
            exit (EXIT_FAILURE);
        }
        /*   */
/*     free(packets[in].pkt_data);
        packets[in].pkt_data = NULL;
*/     
        packets[in].length = pkt_header->caplen;
        packets[in].pkt_header = pkt_header;
        packets[in].pkt_data = malloc ((sizeof (unsigned char)) *(pkt_header->caplen));
        memcpy (packets[in].pkt_data, pkt_data, pkt_header->caplen);

        in = (in+1)%PC_BUFFERSIZE;
        
    }

I can ensure that I have save the value in pkt_data, but It seems not get each value in pkt_header before next call of pcap_next_ex(). the values in packets[in].pkt_header are same.

You should read the documentation for the libpcap functions carefully (emphasis mine):

Originally Posted by man pcap_next_ex

DESCRIPTION


pcap_next_ex(3) reads the next packet and returns a success/failure
indication. If the packet was read without problems, the pointer
pointed to by the pkt_header argument is set to point to the
pcap_pkthdr struct for the packet, and the pointer pointed to by the
pkt_data argument is set to point to the data in the packet. The
struct pcap_pkthdr and the packet data are not to be freed by the
caller, and are not guaranteed to be valid after the next call to
pcap_next_ex(3), pcap_next(), pcap_loop(), or pcap_dispatch(); if the
code needs them to remain valid, it must make a copy of them.

That is mean If I want to save pkt_header, I should allocate new memory to packets[in].header everytime else, right?
like this:

Code:

        packets[in].pkt_header = malloc (sizeof (struct pkt_header));

Well, if you declare pkt_header as a pointer then you need to malloc memory and copy into that memory. Or you can declare pkt_header as an actual struct pkt_header, and just copy into the address of that struct. The plain struct is quicker and less error prone (no need to malloc/free, no bad pointers), but can take up lots of space, and would be wasteful if most packets dont need to have the pkt_header saved. If most/all packets need the pkt_header saved, then make it a plain struct, otherwise, you can choose either way, it wont matter much.

Code:

/* Packet capture buffer */
struct pcbuffer {
     struct pcap_pkthdr *pkt_header;
    u_char *pkt_data;
    int length;    
};
typedef struct pcbuffer packet_capture;
packet_capture packets[PC_BUFFERSIZE];

struct pcap_pkthdr *pkt_header;              /* I defined it as a pointer */
const u_char *pkt_data;

... ...

for (status=0, pcount=0, in=0; (status>=0)&&(pcount<PCOUNT); pcount++) {

        status = pcap_next_ex (phandle, &pkt_header, &pkt_data);

        if (status == 0) {

            /* TIMEOUT */

            continue;

        }

        if (status == -1) {

            printf ("Error reading the packets: %s", pcap_geterr(phandle));

            exit (EXIT_FAILURE);
        }
        /*   */
/*     free(packets[in].pkt_data);
       packets[in].pkt_data = NULL;

*/

        packets[in].length = pkt_header->caplen;
        packets[in].pkt_header = pkt_header;

        packets[in].pkt_data = malloc ((sizeof (unsigned char)) *(pkt_header->caplen));
        memcpy (packets[in].pkt_data, pkt_data, pkt_header->caplen);
 
        in = (in+1)%PC_BUFFERSIZE;
         

    }

I defined it as pointer because pcap_next_ex need the second argument as pcap_pkthdr**

Originally Posted by ppdouble

Hi,
I am newer of learning c programing, I want to know when should I use memcpy() in my program?

Code:

int main() {
    char *a = NULL;
    char *b = NULL;

    b = malloc (sizeof(char) * 5);
    a = b; /*red code */
    memcpy (a, b, sizeof(char)*5); /* blue code */
    
    return (0);
}

Which one is better between red code and blue code?
thx

Dpends if you want a shallow copy (two pointers pointing to the same data) or a deep copy (two versions of the same data). If you change a, do you want and identical change in b?
However you can't memcpy to a null. You need to malloc() a as well as b. The snippet works because you've set a equal to b, and memcpying two identical pointers is a no-op.

Originally Posted by ppdouble

I defined it as pointer because pcap_next_ex need the second argument as pcap_pkthdr**

You don't need to do that in the struct. The way pcap_next_ex probably works is that it uses a static pointer to a struct pcap_pkthdr. In order for you to have a pointer to that struct in your function, you must pass a pointer to the pointer, so pcap_nex_ex can change the contents of your pointer and have that change exist after the function is done. This is how C does "pass by reference". That doesn't mean that you have to use a pointer in your struct pcbuffer. You can declare it as a plain struct pcap_pkthdr and memcpy into that struct. Currently, your code is borken. You don't malloc space for the pkt_header in your packets array, so they all just point to the same static piece of data in pcap_nex_ex. You need to malloc like you showed in post #7, or you need to make the pkt_header member of your struct pcbuffer a plain struct (not a pointer to struct). Your two options (untested, but I think you get the idea):

Code:

/* Packet capture buffer */
struct pcbuffer {
     struct pcap_pkthdr *pkt_header;
    u_char *pkt_data;
    int length;    
};
typedef struct pcbuffer packet_capture;
packet_capture packets[PC_BUFFERSIZE];


struct pcap_pkthdr *pkt_header;              /* I defined it as a pointer */
...
pcap_next_ex(phandle, &pkt_header, &pkt_data);
...
// we have a pointer, but it doesn't point to a valid object, so malloc space for it
packets[in].pkt_header = malloc(sizeof(*(packets[in].pkt_header)));
memcpy(packets[in].pkt_header, pkt_header, sizeof(*(packets[in].pkt_header)));

or

Code:

/* Packet capture buffer */
struct pcbuffer {
    struct pcap_pkthdr pkt_header;  // note, no pointer, a plain struct
    u_char *pkt_data;
    int length;    
};
typedef struct pcbuffer packet_capture;
packet_capture packets[PC_BUFFERSIZE];


struct pcap_pkthdr *pkt_header;  // still a pointer
...
pcap_next_ex(phandle, &pkt_header, &pkt_data);  // this still requires a pointer to a pointer, hence &pkt_header
...
// no need for malloc, we have space for the object since it's a struct and not a pointer
memcpy(&packets[in].pkt_header, pkt_header, sizeof(packets[in].pkt_header));

thx anduril462, I got it.
I have new question.I have two threads. I made packets[in] as buffer, so one thread capture packets and write to the buffer. the other read packets from the buffer(eg. packets[in]). when should I call free()?
one threads call code as follow:

Code:

/* Packet capture buffer */ 
struct pcbuffer {
     struct pcap_pkthdr *pkt_header;
     u_char *pkt_data;
     int length;
     }; 
typedef struct pcbuffer packet_capture;
packet_capture packets[PC_BUFFERSIZE];
struct pcap_pkthdr *pkt_header;
const u_char *pkt_data;
  ... ...  
for (status=0, pcount=0, in=0; (status>=0)&&(pcount<PCOUNT); pcount++) {
         status = pcap_next_ex (phandle, &pkt_header, &pkt_data);
         if (status == 0) {
             /* TIMEOUT */
             continue;
         }
         if (status == -1) {
             printf ("Error reading the packets: %s", pcap_geterr(phandle));
             exit (EXIT_FAILURE);
         }
        pthread_mutex_lock (&mutex);
         /* Should I call free() here?????   */
 /*     free(packets[in].pkt_data);
        packets[in].pkt_data = NULL;
        free(packets[in].pkt_header);
        packets[in].pkt_header = NULL; */
        packets[in].length = pkt_header->caplen;
        packets[in].pkt_data = malloc ((sizeof (unsigned char)) * (pkt_header->caplen));
        memcpy (packets[in].pkt_data, pkt_data, pkt_header->caplen);
        packets[in].pkt_header = malloc(sizeof(*(packets[in].pkt_header)));

        memcpy(packets[in].pkt_header, pkt_header, sizeof(*(packets[in].pkt_header)));
           pthread_mutex_unlock (&mutex);

            in = (in+1)%PC_BUFFERSIZE; 
           }

should I call free() in line 23?
the other one call code as follow:

Code:

    pcap_pkthdr *pkt_header;
    u_char *pkt_data;
    int length;
    int out = 0;
    while (1) {
        /* Read from buffer (critical area) */
        pthread_mutex_lock (&mutex);
        if (packets[out].pkt_data == NULL) {
            pthread_mutex_unlock (&mutex);
            continue;
        }
        else {
            pkt_header = packets[out].pkt_header;
            pkt_data = packets[out].pkt_data;
            length = packets[out].length;
        }
        pthread_mutex_unlock (&mutex);
        /* end of read (critical area) */

        out = (out+1) % PC_BUFFERSIZE;
    }

If I call free() in the former codes ,can I got the right value in the point pkt_header in the latter(line 13) codes?

You only call free when you are all done with that memory and never want to access it again. If you try to access memory you have freed, the result is undefined behavior.

Your essentially have a producer-consumer model where the two can share memory. In your scenario, the producer would allocate memory when it gets a new packet, and put that packet in the queue for processing by the consumer. The consumer would take that packet out of the queue and do something with it. Then, when the consumer is all done, it would free it. That top code is your producer, it probably shouldn't be freeing anything. You need to free in the consumer (the bottom code) when you're done with that packet.

well, If the buffer is full, and it will be rewrite by the producer from in=0 again.
imagine that in equals to 5, but consumer didnt copy it before the producer call malloc again. and now the packets[5].pkt_data will be rewrite. what will happen? because last memory allocated by malloc() last time is not freed at this time.

Code:

        packets[in].pkt_data = malloc ((sizeof (unsigned char)) * (pkt_header->caplen));        
        memcpy (packets[in].pkt_data, pkt_data, pkt_header->caplen);
        packets[in].pkt_header = malloc(sizeof(*(packets[in].pkt_header)));
 
        memcpy(packets[in].pkt_header, pkt_header, sizeof(*(packets[in].pkt_header)));
        pthread_mutex_unlock (&mutex); 

        in = (in+1)%PC_BUFFERSIZE;

Typically in a producer-consumer model, if the queue is full, the producer stops and waits for the consumer to consumer some data so there is room again. You typically don't want to just overwrite or discard data.

Regardless of whether you follow that model, or the "just throw away data" model, the rule is still: only free memory when you are all done with it and never need to access it again. It's really that simple. If the consumer is done with it, free it. If the producer is throwing it away because the consumer is taking too long, free it.