Anfängerproblem C & malloc

[BrainPain]

Hallo Leute,

ich glaube ich habe ein grundlegendes Verständnisproblem was Speicherreservierung unter C angeht.

Erstmal meinen Code (testprog.c):

#include <stdio.h>
#include <stdlib.h>
#include <string.h>


void stringTest(char const* src, char* dst){
	char *sptr = malloc(sizeof(char)*((strlen(src)*2)+1));
	char c;
	do{
		c = *src++;
		*sptr++ = c;
		*sptr++ = ' ';
	}while(c);

	dst = malloc(sizeof(char)*(strlen(sptr)+1));

	strcpy(dst, sptr);
	free(sptr);
}


int main(void){
	char *destWord;
	stringTest("TestString", destWord);
	printf("%s\n",destWord);
	return 0;
}

Und die Ausgabe:

# gcc -Wall testprog.c -o testprog
# ./testprog
testprog in free(): warning: modified (chunk-) pointer
Speicherschutzverletzung(core dumped)

Wie schreibe ich in den reservierten Speicher? Wenn ich ein Array (also mit konstanter Länge) in die Funktion übergebe funktioniert es. Zeigt der zeiger nach Aufruf von malloc auf die falsche Stelle? Oder liegt das Problem an ganz anderer Stelle?

Viele Grüße

 

[Tron]

sptr zeigt anfänglich auf den alloziierten Speicher. In der Schleife veränderst du sptr (sptr++), wodurch es dann natürlich nicht mehr auf die ursprüngliche Stelle zeigt.

Anbei: Deine Schleife verursacht einen Pufferüberlauf um ein Zeichen. Wozu dient weiter das Kopieren der neu erstellten Zeichenkette? (Wofür es übrigens die Funktion strdup() gibt)

Nachsatz: Mir fällt auch gerade auf, dass der zweite Parameter nicht das bewirkt, was du denkst. Das setzen von dst in der Funktion hat keinerlei Auswirkung auf den Wert von destWord im Aufrufer. Parameter werden _immer_ per Wertübergabe an eine Funktion übergeben. Wenn du einen Zeiger als Resultat liefern willst, dann musst du das entweder in Form eines Rückgabewertes bewerkstelligen (char* stringTest(...)) oder einen Zeiger auf einen Zeiger übergeben (void charTest(char** z) { *z = ... } [...] { char* res; charTest(&res); }).

 

[BrainPain]

Nachsatz: Mir fällt auch gerade auf, dass der zweite Parameter nicht das bewirkt, was du denkst. Das setzen von dst in der Funktion hat keinerlei Auswirkung auf den Wert von destWord im Aufrufer. Parameter werden _immer_ per Wertübergabe an eine Funktion übergeben. Wenn du einen Zeiger als Resultat liefern willst, dann musst du das entweder in Form eines Rückgabewertes bewerkstelligen (char* stringTest(...)) oder einen Zeiger auf einen Zeiger übergeben (void charTest(char** z) { *z = ... } [...] { char* res; charTest(&res); }).

Genau das war es! Vielen Dank, Tron.

Lösung:

#include <stdio.h>
#include <stdlib.h>
#include <string.h>


void stringTest(char const* src, [COLOR="Red"]char** dst[/COLOR]){
	char *sptr = malloc(sizeof(char)*((strlen(src)*2)+1));
	[COLOR="Red"]char *oldPos = sptr;[/COLOR]
	char c;
	do{
		c = *src++;
		*sptr++ = c;
		*sptr++ = ' ';
	}while(c);

	[COLOR="Red"]*dst = strdup(oldPos);[/COLOR]
	free([COLOR="Red"]oldPos[/COLOR]);
}


int main(void){
	char* destWord;
	stringTest("TestString", [COLOR="Red"]&destWord[/COLOR]);
	printf("%s\n",destWord);
	return 0;
}

 

[Tron]

Der Pufferüberlauf in der Schleife besteht weiterhin.
Die Kopie der Zeichenkette erscheint mir immernoch sinnlos.

 

[Xantus]

edit: hier stand mist, hab das strdup überlesen
aber da fällt mir ein:
http://www.freebsd.org/cgi/man.cgi?...ion=0&manpath=FreeBSD+6.3-RELEASE&format=html
ein free vor return 0; sollte nicht schaden

 

[BrainPain]

Der Pufferüberlauf in der Schleife besteht weiterhin.
Die Kopie der Zeichenkette erscheint mir immernoch sinnlos.

Ich bin für Tipps immer dankbar. Wo siehst du denn den Pufferüberlauf? Ich habe ja (2*Länge)+1 allokiert. Müsste doch reichen für die doppelte Stringlänge + '\0', oder nicht?

Das Kopieren der Zeichenkette mache ich um es an das aufrufende Programm zurück zu geben, da ich den Rückgabewert schon für die Stringlänge nutze was in diesem Beispiel nicht zu sehen ist. In der eigentlichen Funktion hänge ich nur bei bestimmten Zeichen ein weiteres Zeichen an, wodurch ich die endgültige Länge nicht kenne. Deswegen nutze ich strdup um einen String zu erhalten der die genaue Länge hat und nicht die "Worst-Case"-Länge (2*Stringlänge).

Ich habe gelesen das strdup nicht zum ANSI-Standard gehört. Wie könnte man es ersetzen? So wie ich es im ersten Versuch getan habe klappt es scheinbar nicht.

ein free vor return 0; sollte nicht schaden

Da hast du natürlich recht, aber das "main" dient hier nur zu Testzwecken von daher ist es wohl egal.

Viele Grüße

 

[xbit]

Wenn Dein c = *src++ ausgefuehrt wird, tritt ein Ueberlauf auf.
Wenn *src auf die abschliessende \0 zeigt, inkrementierst Du danach den Pointer und er zeigt auf das Byte dahinter, das nicht mehr zum String gehoert.

strdup() koenntest Du so implementieren:

void simple_strdup(const char *const src, char **dst) {
  size_t len = strlen(src) + 1;
  *dst = calloc(len, sizeof(char));
  strncpy(*dst, src, len);
}

Wobei der Aufrufer den Speicher freigeben muss.

Ich wuerde es eher so implementieren, dass man den Speicher vor dem Aufruf holen muss und dann die Laenge von **dst als zusaetzlichen Parameter angibt. Das spart auch eine Indirektion, weil char *dst als 2. Parameter reicht.

HTH

 

[Xantus]

Da hast du natürlich recht, aber das "main" dient hier nur zu Testzwecken von daher ist es wohl egal.

Das hat doch damit nix zu tun. Reservierter Speicher ist nunmal reservierter Speicher, der freigegeben werden muss. Und wenn du 1000mal testest hast du irgendwann genug für dich beantsprucht, dass das System dich nicht mehr mag.
genauso wie } zu { gehört, gehört free zu malloc/calloc/realloc. bring es deinen fingern besser gleich bei

grüße, xantus

 

[Tron]

Ich bin für Tipps immer dankbar. Wo siehst du denn den Pufferüberlauf? Ich habe ja (2*Länge)+1 allokiert. Müsste doch reichen für die doppelte Stringlänge + '\0', oder nicht?

Das ' ', das direkt nach dem Kopieren des '\0' geschrieben wird, verursacht einen Pufferüberlauf. Denk dir als Beispiel als Eingabe eine leere Zeichenkette, also nur aus dem abschließenden \0 bestehend.

Das Kopieren der Zeichenkette mache ich um es an das aufrufende Programm zurück zu geben, da ich den Rückgabewert schon für die Stringlänge nutze was in diesem Beispiel nicht zu sehen ist. In der eigentlichen Funktion hänge ich nur bei bestimmten Zeichen ein weiteres Zeichen an, wodurch ich die endgültige Länge nicht kenne. Deswegen nutze ich strdup um einen String zu erhalten der die genaue Länge hat und nicht die "Worst-Case"-Länge (2*Stringlänge).

Ob sich das lohnt...

Ich habe gelesen das strdup nicht zum ANSI-Standard gehört. Wie könnte man es ersetzen? So wie ich es im ersten Versuch getan habe klappt es scheinbar nicht.

Es ist SUSv2, das sollte gut genug sein.

 

[Tron]

Wenn Dein c = *src++ ausgefuehrt wird, tritt ein Ueberlauf auf.
Wenn *src auf die abschliessende \0 zeigt, inkrementierst Du danach den Pointer und er zeigt auf das Byte dahinter, das nicht mehr zum String gehoert.

Falsch, das ist kein Pufferüberlauf. Es ist wohldefiniert direkt (1) hinter das Ende einer Reihung zu zeigen. Lesen oder schreiben darauf ist natürlich nicht erlaubt, aber das geschieht auch nicht. Siehe meinen vorhergehenden Beitrag, worin der Pufferüberlauf besteht.

strdup() koenntest Du so implementieren:

void simple_strdup(const char *const src, char **dst) {
  size_t len = strlen(src) + 1;
  *dst = calloc(len, sizeof(char));
  strncpy(*dst, src, len);
}

Verwende nie strncpy(). Diese Funktion ist einfach Mist. Sie garantiert nicht das Schreiben eines abschließenden \0.
Auch ist zur Verwendung von Rückgabewerten statt Ausgabeparameter zu raten.

Ich wuerde es eher so implementieren, dass man den Speicher vor dem Aufruf holen muss und dann die Laenge von **dst als zusaetzlichen Parameter angibt. Das spart auch eine Indirektion, weil char *dst als 2. Parameter reicht.

Worin besteht der Sinn? Dann vollführt der strdup-Ersatz noch genau ein strcpy().

 

[Tron]

Das hat doch damit nix zu tun. Reservierter Speicher ist nunmal reservierter Speicher, der freigegeben werden muss. Und wenn du 1000mal testest hast du irgendwann genug für dich beantsprucht, dass das System dich nicht mehr mag.
genauso wie } zu { gehört, gehört free zu malloc/calloc/realloc. bring es deinen fingern besser gleich bei

Das ist schlicht falsch. Mit dem Beenden eines Prozesses werden alle von ihm belegten Ressourcen, also insbesondere Speicher, freigegeben.

 

[troll]

Das ist schlicht falsch. Mit dem Beenden eines Prozesses werden alle von ihm belegten Ressourcen, also insbesondere Speicher, freigegeben.

Das ist allerdings nicht so definiert.
Jedes mir bekannte moderne OS macht es so, aber mehr aus Nettigkeit. ;-)

 

[Xantus]

korrekt, und deswegen ist Trons Aussage schlicht falsch.
Jedes moderne OS kann es machen, aber auch nicht immer. Probiers mal aus, wenn du an ner Windows Kiste sitzt :]

 

[troll]

korrekt, und deswegen ist Trons Aussage schlicht falsch.
Jedes moderne OS kann es machen, aber auch nicht immer. Probiers mal aus, wenn du an ner Windows Kiste sitzt :]

Also macht das OS das nicht aus Nettigkeit, sondern aus Mitleid.
*duck*