Eigene memcpy Implementierung crasht

Hi, warum crasht denn bei folgender memcpy implementierung das Programm bzw. es kommt immer die Meldung, dass der Stack um items currupiert worden wäre.

#include <iostream>

void Copy(void* sourceArray, void* destinationArray, int count)
{
	while (count--)
	{
		*(char*)destinationArray = *(char*)sourceArray;
		destinationArray = (char*)destinationArray + 1;
		sourceArray = (char*)sourceArray + 1;
	}
}

int _tmain(int argc, _TCHAR* argv[])
{
	int* items = new int[10];
	int* newitems = new int[10];

	items[0] = 1;
	items[1] = 2;
	items[2] = 3;
	items[3] = 4;
	items[4] = 5;
	items[5] = 6;
	items[6] = 7;
	items[7] = 8;
	items[8] = 9;
	items[9] = 10;

	Copy(&items, &newitems, 10);

	for (int i = 0; i < 10; ++i)
	{
		std::wcout << newitems[i] << std::endl;
	}

	std::wcout << L"Press any key to continue." << std::endl;
	std::wcin.get();
	delete[] items;
	delete[] newitems;
	return 0;
}

Sone

Wieso zum Teufel so viele Pointer und Freispeicherallokierung? Kein Wunder das ist C mit cin und cout (und new und delete ).

Sieh dir mal dynamische/statische Arrays in C++ an. Da nutzt man die STL-Container. Dazu gibt es passendes Zubehör (STL-Algorithmen). Dann steht schon kein new / delete mehr in deinem Programm. Und dann wirst du auch nie wieder memcpy benutzen.

P.S.: Die casts die du da durchführst sind auch fragwürdig. Ich frag mich, ob das UB ist....

SeppJ

Zeile 29: Du gibst der Funktion die Adresse der Zeiger. Höchstwahrscheinlich nicht das was du möchtest.

Beim Querlesen habe ich noch vieles anderes verdächtiges und gefährliches gesehen, aber bevor ich ins Detail gehe: Wozu soll das überhaupt gut sein?

Dein Copy an sich sollte übrigens richtig sein, auch wenn man das sehr leicht wesentlich schöner machen könnte, zum Beispiel durch const-correctness und hübschere Schreibweise:

void my_memcpy(void* dest, const void* src, size_t count) 
{
 char* char_dest = (char*)dest;  // Oder C++-Casts, damit Sone keinen Anfall bekommt
 char* char_src = (char*)src;

 while (count--)
  *char_dst++ = *char_src++;  
}

Macht genau das gleiche wie deine Variante.

edit:

Sone schrieb:

P.S.: Die casts die du da durchführst sind auch fragwürdig. Ich frag mich, ob das UB ist....

Die sollten in Ordnung sein. Was stört dich an denen? Oder ist das einfach dein anerzogener Beißreflex, wenn du einen C-Cast siehst?

edit2: Schreibfehler im Code korrigiert.

Copy(items, newitems, 10 * sizeof(int));
// bzw.
Copy(items, newitems, 10 * sizeof(*items));

Und kürzer gehts auch noch (schöner ist es dadurch aber nicht):

for (char* char_dst = (char*)dst, *char_src = (char*)src; count--;) 
 *char_dst++ = *char_src++;

cooky451

Du hast ein * vergessen im Code SeppJ.

SeppJ

cooky451 schrieb:

Du hast ein * vergessen im Code SeppJ.

Ups! Danke! Das kommt davon, wenn man im Editfenster programmiert.

Sone

SeppJ schrieb:

Sone schrieb:

P.S.: Die casts die du da durchführst sind auch fragwürdig. Ich frag mich, ob das UB ist....

Die sollten in Ordnung sein. Was stört dich an denen? Oder ist das einfach dein anerzogener Beißreflex, wenn du einen C-Cast siehst?

Nein, nein. Ich meinte dieser Cast von int* nach void* und dann nach char* - ist das so vollkommen in Ordnung?

Shade Of Mine

Sone schrieb:

SeppJ schrieb:

Sone schrieb:

P.S.: Die casts die du da durchführst sind auch fragwürdig. Ich frag mich, ob das UB ist....

Die sollten in Ordnung sein. Was stört dich an denen? Oder ist das einfach dein anerzogener Beißreflex, wenn du einen C-Cast siehst?

Nein, nein. Ich meinte dieser Cast von int* nach void* und dann nach char* - ist das so vollkommen in Ordnung?

Warum sollte es das nicht sein?

pumuckl

Sone schrieb:

ist das so vollkommen in Ordnung?

Ja. Nach void* und char* darf immer gecastet werden.

Sone

Keine Ahnung. Ich habe einen Beißreflex auf Pointer-Casts. Das sieht mir einfach immer so eklig aus

Ethon

Anders wäre memcpy sowieso nicht vernünftig implementierbar.

hustbaer

Sone schrieb:

Keine Ahnung. Ich habe einen Beißreflex auf Pointer-Casts. Das sieht mir einfach immer so eklig aus

Womit wir wieder beim Thema wären: einfach mal die Fresse halten wenn man nix zu sagen hat.

Sone

hustbaer schrieb:

Sone schrieb:

Keine Ahnung. Ich habe einen Beißreflex auf Pointer-Casts. Das sieht mir einfach immer so eklig aus

Womit wir wieder beim Thema wären: einfach mal die Fresse halten wenn man nix zu sagen hat.

Ähm... und was hat das mit mir zu tun? Ich hab' die Frage gestellt, ob das ein gültiger Cast ist. Ich habe die Antwort bekommen. Und du übertreibst wieder in alle Maßen? Was los bei dir?

nurf

Ab SSE2 empfehle ich folgendes.

void nontemporal_copy(char* outbuff, char* inbuff, int size)
{
const int step = 64; // cache line
while(size > step) {
  _mm_prefetch(inbuff + 320, _MM_HINT_NTA); // non-temporal prefetch
  __m128i A = _mm_loadu_si128((__m128i*) (inbuff +  0));
  __m128i B = _mm_loadu_si128((__m128i*) (inbuff + 16));
  __m128i C = _mm_loadu_si128((__m128i*) (inbuff + 32));
  __m128i D = _mm_loadu_si128((__m128i*) (inbuff + 48));

  // destination address must be 16-byte aligned!
  _mm_stream_si128((__m128i*) (outbuff +  0), A);
  _mm_stream_si128((__m128i*) (outbuff + 16), B);
  _mm_stream_si128((__m128i*) (outbuff + 32), C);
  _mm_stream_si128((__m128i*) (outbuff + 48), D);

  inbuff  += step;
  outbuff += step;
  size -= step;
}
_mm_mfence();
}