Thread: when should I use memcpy()?

  1. #1
    Registered User
    Join Date
    May 2012
    Posts
    33

    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

  2. #2
    C++ Witch laserlight's Avatar
    Join Date
    Oct 2003
    Location
    Singapore
    Posts
    28,413
    What are you trying to do?
    Quote Originally Posted by Bjarne Stroustrup (2000-10-14)
    I get maybe two dozen requests for help with some sort of programming or design problem every day. Most have more sense than to send me hundreds of lines of code. If they do, I ask them to find the smallest example that exhibits the problem and send me that. Mostly, they then find the error themselves. "Finding the smallest program that demonstrates the error" is a powerful debugging tool.
    Look up a C++ Reference and learn How To Ask Questions The Smart Way

  3. #3
    Registered User
    Join Date
    Nov 2010
    Location
    Long Beach, CA
    Posts
    5,909
    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.

  4. #4
    Registered User ledow's Avatar
    Join Date
    Dec 2011
    Posts
    435
    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).

    - Compiler warnings are like "Bridge Out Ahead" warnings. DON'T just ignore them.
    - A compiler error is something SO stupid that the compiler genuinely can't carry on with its job. A compiler warning is the compiler saying "Well, that's bloody stupid but if you WANT to ignore me..." and carrying on.
    - The best debugging tool in the world is a bunch of printf()'s for everything important around the bits you think might be wrong.

  5. #5
    Registered User
    Join Date
    May 2012
    Posts
    33
    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.

  6. #6
    Registered User
    Join Date
    Nov 2010
    Location
    Long Beach, CA
    Posts
    5,909
    You should read the documentation for the libpcap functions carefully (emphasis mine):
    Quote 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.

  7. #7
    Registered User
    Join Date
    May 2012
    Posts
    33
    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));

  8. #8
    Registered User
    Join Date
    Nov 2010
    Location
    Long Beach, CA
    Posts
    5,909
    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.

  9. #9
    Registered User
    Join Date
    May 2012
    Posts
    33
    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**

  10. #10
    Registered User
    Join Date
    May 2012
    Posts
    505
    Quote Originally Posted by ppdouble View Post
    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.

  11. #11
    Registered User
    Join Date
    Nov 2010
    Location
    Long Beach, CA
    Posts
    5,909
    Quote Originally Posted by ppdouble View Post
    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));

  12. #12
    Registered User
    Join Date
    May 2012
    Posts
    33
    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?
    Last edited by ppdouble; 05-31-2012 at 08:52 PM.

  13. #13
    Registered User
    Join Date
    Nov 2010
    Location
    Long Beach, CA
    Posts
    5,909
    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.

  14. #14
    Registered User
    Join Date
    May 2012
    Posts
    33
    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;

  15. #15
    Registered User
    Join Date
    Nov 2010
    Location
    Long Beach, CA
    Posts
    5,909
    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.

Popular pages Recent additions subscribe to a feed

Similar Threads

  1. using memcpy?
    By nyekknyakk in forum C Programming
    Replies: 1
    Last Post: 08-19-2010, 07:47 PM
  2. memcpy
    By m37h0d in forum C++ Programming
    Replies: 28
    Last Post: 04-12-2008, 09:10 PM
  3. memcpy
    By mbooka in forum C Programming
    Replies: 10
    Last Post: 02-28-2006, 02:25 AM
  4. memcpy use?
    By jerrykelleyjr in forum C++ Programming
    Replies: 21
    Last Post: 02-17-2005, 07:39 AM
  5. Help: About memcpy()
    By naruto in forum C Programming
    Replies: 41
    Last Post: 06-25-2004, 03:47 PM