Memmove making one spot garbage?

Question

I'm using memmove to shift std::string elements to the right by one spot. The first spot of the destination is the only spot that gets messed up and filled with garbage. I am using memmove instead of strcpy because I am using it in a class that requires templates. It works fine when I have an array of ints though. Any idea on how to fix this for strings?

/**
 * @brief Shifts elements in the array to the right
 * @param newItem The insert position.
 */
template<class T>
void DynamicArray<T>::shiftElementsRight(uint newItem) {
    uint diff = _size - newItem;
    memmove(&(_array[newItem + 1]),
            &(_array[newItem]), sizeof(_array[0]) * diff);
}

My Main

int main() {
    DynamicArray<string> array(1);

    string val1 = "val1";
    array.add(val1);
    string val2 = "val2";
    array.add(val2);
    // ... more additions ...

The last output:

Sizeof: 8
Value is: val1
Value is: val11val3, val21
                            ����[val1, val2A����[val1, val2, val31val4, Aq�����[val1, val2, val3 val4, val5Q`r�#����[val1, val2, val3, val4, val5, val61val5, 1val6, Q"����[val1, val2, val3, al4, a
8s��p�hp��p�hq�(r�Xr�؄KC�؄KC�؄KC�1val7, a:����[val1, val2, val3, val4, val5, qF����[val1, val2, val3, val4, val5, val6]

score 6 · Accepted Answer · answered Oct 18 '13 at 07:37

Neither memcpy nor memmove are possible when your array contains std::string objects. The reason is that both tamper with the objects' internals when they shouldn't (the integrity of a C++ object is ensured by its constructor / assignment operator / destructor / member functions, you must not modify its bytes manually unless you know exactly what you do).

As already suggested, use a method that takes constructors, assignment operators and destructors into account (std::copy_backward for example) or better yet, get rid of those legacy C arrays and use proper C++ containers like vector and use the container's functions to insert/remove items.

In other words, as soon as you start using C++ objects then you have to begin writing C++, not "C with classes" any more.

score 2 · Answer 2 · edited Oct 18 '13 at 07:09

2

From memcpy description:

To avoid overflows, the size of the arrays pointed by both the destination and source parameters, shall be at least num bytes, and should not overlap (for overlapping memory blocks, memmove is a safer approach).

Use memmove, in other words.

edited Oct 18 '13 at 07:09

Useless

64,155
6
88
132

answered Oct 18 '13 at 07:07

Ivan Ishchenko

1,468
9
12

score 1 · Answer 3 · answered Oct 18 '13 at 07:23

Like most people here say you can use memmove instead of memcpy because it can handle overlapping ranges.

However, the C++ solution would be to use copy_backward, which does explicitly what you asked for and will behind the screens choose the fastest copy method for you. That would look something like this:

std::copy_backward(std::begin(_array)+newItem, std::end(_array)-1, std::end(array));

This means you will copy backwards from the previous-to-last element to position newItem and will insert that starting from the back. This can obviously handle the overlap this requires. It might be that behind the screens memmove or other platform-specific memory copies are used.

score 0 · Answer 4 · answered Oct 18 '13 at 07:08

 SYNOPSIS
        #include <string.h>

        void *memcpy(void *dest, const void *src, size_t n);

 DESCRIPTION
        The  memcpy()  function  copies  n bytes from memory area src to memory
        area dest.  The memory areas must not overlap.  Use memmove(3)  if  the
        memory areas do overlap.

Your memory areas must not overlap, otherwise memcpy's behavior is not defined. You should use memmove instead.

score 0 · Answer 5 · edited May 23 '17 at 10:31

You may not use memcpy for overlapping ranges, as is stated in the library documentation:

7.21.2 Copying functions

7.21.2.1 The memcpy function

...

2 The memcpy function copies n characters from the object pointed to by s2 into the object pointed to by s1. If copying takes place between objects that overlap, the behavior is undefined.

7.21.2.2 The memmove function

...

2 The memmove function copies n characters from the object pointed to by s2 into the object pointed to by s1. Copying takes place as if the n characters from the object pointed to by s2 are first copied into a temporary array of n characters that does not overlap the objects pointed to by s1 and s2, and then the n characters from the temporary array are copied into the object pointed to by s1.

For POD or trivially initialized types, you can either use memmove (as suggested above) or std::copy_backward (so long as the last destination element is not within the source range)

This question has more discussion of the memcpy-vs-memmove part.

For non-trivial types such as std::string, you can't use either memcpy or memmove, but must use std::copy or std::copy_backward.

Memmove making one spot garbage?

5 Answers5