(C) seltsamer calloc-Fehler + Pointerfragen

[*Markus]

Hallo,

ich habe hier ein seltsames Verhalten und hoffe, dass es jemand erklären kann. Ich habe zwei Funktionen, setMetaDataBezeichnung(struct metadata **m, const char *s) und setMetaDataZusatzInfo(struct metadata **m, const char *s):

=== Main.c ===

  
#include <stdio.h>  
#include <stdlib.h> 					//malloc  
#include "metadata.h"  
  
int main(void)   {  
  
    MetaData m1 = (MetaData)malloc(sizeof(MetaData));  
  
    setMetaDataBezeichnung(&m1, "Das ist die erste Bezeichnung");  
    setMetaDataZusatzInfo(&m1, "Das ist eine zusätzliche Information");  
    printf("%s\n", getMetaDataBezeichnung(m1));  
    printf("%s\n", getMetaDataZusatzInfo(m1));  
    free(m1);  
  
return 0;  
}  

=== metadata.h ====

  
#ifndef METADATA_H  
#define METADATA_H  
  
#define MAX_INFO 100  
#define MAX_BEZ 50  
  
typedef struct metadata *MetaData;  
  
void setMetaDataBezeichnung(struct metadata **m, const char *s);  
void setMetaDataZusatzInfo(struct metadata **m, const char *s);  
char* getMetaDataBezeichnung();  
char* getMetaDataZusatzInfo();  
  
#endif  
  
=== Metadata.c ====  
#include <stdio.h>  
#include <string.h>			//strlen, strncpy  
#include <stdlib.h>			//calloc  
#include "metadata.h"  
  
struct metadata  {  
	char *bezeichnung;  
	char *zusatzinfo;  
};  
  
void setMetaDataBezeichnung(struct metadata **m, const char *s)  {	  
	 //Falls Zeichenkette länger als MAX_BEZ ist, nur MAX_BEZ+1 (+1 wg. \0) allozieren.  
	 if (strlen(s) >= MAX_BEZ)  
	    (*m)->bezeichnung = (char*)calloc(MAX_BEZ+1,sizeof(char));  
	 else  
	 	 (*m)->bezeichnung = (char*)calloc(strlen(s)+1,sizeof(char));  
	 	  
	 if ((*m)->bezeichnung == 0)  {  
	 	 printf("Fehler bei der Speicherallozierung");  
	 	 exit(1);  
	 }  
	 //Maximal MAX_BEZ Zeichen kopieren. \0 wird automatisch angehängt, da +1 noch frei ist.  
	 strncpy((*m)->bezeichnung, s, MAX_BEZ);  
}  
  
void setMetaDataZusatzInfo(struct metadata **m, const char *s)   {  
	 //Falls Zeichenkette länger als MAX_INFO ist, nur MAX_INFO+1 (+1 wg. \0) allozieren.  
	 if (strlen(s) >= MAX_INFO)  
	    (*m)->zusatzinfo = (char*)calloc(MAX_INFO+1,sizeof(char));  
	 else  
	 	 (*m)->zusatzinfo = (char*)calloc(strlen(s)+1,sizeof(char));  
	 	  
	 if ((*m)->zusatzinfo == 0)  {  
	 	 printf("Fehler bei der Speicherallozierung");  
	 	 exit(1);  
	 }  
	 //Maximal MAX_INFO Zeichen kopieren. \0 wird automatisch angehängt, da +1 noch frei ist.  
	 strncpy((*m)->zusatzinfo, s, MAX_INFO);  
}  
  
char* getMetaDataBezeichnung(struct metadata *m)   {  
	 return m->bezeichnung;  
}  
  
char* getMetaDataZusatzInfo(struct metadata *m)   {  
	 return m->zusatzinfo;  
}  
  

Diese Funktionen sind praktisch identisch. Ich kompiliere mit ...

$ gcc Metadata.c Main.c -o Main -std=c99

...was auch durchläuft, aber führe ich nun ./Main aus, bekomme ich seltsamerweise folgende Ausgabe:

markus@archy ~/C-Programme/Projekte/Dateibrowser $ ./Main
Main: malloc.c:3074: sYSMALLOc: Assertion `(old_top == (((mbinptr) (((char *) &((av)->bins[((1) - 1) * 2])) - __builtin_offsetof (struct malloc_chunk, fd)))) && old_size == 0) || ((unsigned long) (old_size) >= (unsigned long)((((__builtin_offsetof (struct malloc_chunk, fd_nextsize))+((2 * (sizeof(size_t))) - 1)) & ~((2 * (sizeof(size_t))) - 1))) && ((old_top)->size & 0x1) && ((unsigned long)old_end & pagemask) == 0)' failed.
Abgebrochen

Was hat das zu bedeuten?
Ironischerweise funktioniert das Ganze aber, wenn ich die Funktion setMetaDataZusatzInfo kommentiere und somit nicht benutze. Wird also nur die erste Funktion aufgerufen funktioniert das Programm und ich bekomme die Ausgabe "Das ist die erste Bezeichnung". Kann sich das jemand erklären?

Meine zweite Frage betrifft die Pointerzuweisung. In metadata.h schreibe ich typedef struct metadata *MetaData; Ich kann nicht typedef struct metadata MetaData; schreiben, damit dieses teils objektorientierte Paradigma funktioniert (eigentlich wollte ich versuchen, die Struktur auf dem Stack zu erzeugen, aber das scheint irgendwie nicht zu funktionieren, da ja dann die Größe der Struktur nicht bekannt ist).
In Main.c ist m1 somit ein Pointer auf eine Struktur MetaData. In &m1 übergebe ich somit die Adresse des Pointers. Um zur Struktur zu gelangen, muss ich in setMetaDataBezeichnung **m offensichtlich doppelt dereferenzieren. Liegt offensichtlich daran, dass der Zeiger nicht die Struktur selbst ist, sondern nur auf die Struktur (die irgendwo im Speicher existiert) zeigt. Ist das richtig so? Ich kann die (funktionierende) erste Funktion setMetaDataBezeichnung auch so hinbiegen, dass ich in der Funktion keine doppelten Referenzen benötige, muss dann aber *&m1 übergeben. Ich mache das immer nach Gefühl, und es funktioniert meistens, aber ich habe da irgendwie noch Vorstellungsschwierigkeiten. *&m1 wäre dann praktisch die dereferenzierte Adresse des Pointer, was dann praktisch die Struktur selbst ist. Deswegen funktioniert dies dann (mit der angepassten Funktion) auch. Ist das richtig so?

Eine letzte Frage hätte ich noch zum Freigeben des Speichers. In Main führe ich ein free(m1) aus, was aber nicht ganz sauber wäre. Ich müsste die einzelnen Variablen der Struktur einzeln für sich auch freigeben, da ein killen der Struktur ja nicht heißt, dass auch die Variablen freigegeben wurden (da sie ja wieder nur Zeiger sind, die irgendwo Platz reserviert haben, der aber selbst auch wieder freigegeben werden muss). Ist das richtig so?

Danke,
Markus

[Alexander (HH)]

Moin Moin!

void setMetaDataBezeichnung(struct metadata **m, const char *s);
void setMetaDataZusatzInfo(struct metadata **m, const char *s);

Entweder deutsch oder englisch, aber nicht so durcheinander. Warum struct metadata **m? Im Code wäre ein struct metadata *m wesentlich einfacher, denn Du änderst m ja nicht.

char* getMetaDataBezeichnung();
char* getMetaDataZusatzInfo();

Diese beiden Prototypen passen nicht zur Funktion weiter unten.

Ich kompiliere mit ...

$ gcc Metadata.c Main.c -o Main -std=c99

-Wall -pedantic sind oft hilfreich.

markus@archy ~/C-Programme/Projekte/Dateibrowser $ ./Main
Main: malloc.c:3074: sYSMALLOc: Assertion `(old_top == (((mbinptr) (((char *) &((av)->bins[((1) - 1) * 2])) - __builtin_offsetof (struct malloc_chunk, fd)))) && old_size == 0) || ((unsigned long) (old_size) >= (unsigned long)((((__builtin_offsetof (struct malloc_chunk, fd_nextsize))+((2 * (sizeof(size_t))) - 1)) & ~((2 * (sizeof(size_t))) - 1))) && ((old_top)->size & 0x1) && ((unsigned long)old_end & pagemask) == 0)' failed.
Abgebrochen

Was hat das zu bedeuten?

Ein Paranoia-Test in malloc ist fehlgeschlagen. Du ruft offenbar malloc() mit Parametern auf, die dem malloc()-Code nicht gefallen.

Ironischerweise funktioniert das Ganze aber, wenn ich die Funktion setMetaDataZusatzInfo kommentiere und somit nicht benutze. Wird also nur die erste Funktion aufgerufen funktioniert das Programm und ich bekomme die Ausgabe "Das ist die erste Bezeichnung". Kann sich das jemand erklären?

Verbogene Zeiger, Syntaxfehler, ...

Fang mit -Wall -pedantic an.

Meine zweite Frage betrifft die Pointerzuweisung. In metadata.h schreibe ich typedef struct metadata *MetaData; Ich kann nicht typedef struct metadata MetaData; schreiben, damit dieses teils objektorientierte Paradigma funktioniert (eigentlich wollte ich versuchen, die Struktur auf dem Stack zu erzeugen, aber das scheint irgendwie nicht zu funktionieren, da ja dann die Größe der Struktur nicht bekannt ist).

Doch, Du definierst die struct metadata nur an der falschen Stelle, im C-Code statt im Header.

Eine letzte Frage hätte ich noch zum Freigeben des Speichers. In Main führe ich ein free(m1) aus, was aber nicht ganz sauber wäre. Ich müsste die einzelnen Variablen der Struktur einzeln für sich auch freigeben, da ein killen der Struktur ja nicht heißt, dass auch die Variablen freigegeben wurden

Richtig. Wenn Du Dich nicht wie ein Assembler-Programmierer mit der Speicherverwaltung rumschlagen willst, nimm Ruby, Perl oder ähnliches. Ansonsten initialisiere deine frische struct metadata so, dass alle später allozierten Pointer erst einmal auf NULL gesetzt sind. Schreibe eine freeMetadata()-Methode, die alle Zeiger-Elemente der Struktur, die nicht NULL sind, wieder freigibt.

Alexander

[*Markus]

Hallo,

Entweder deutsch oder englisch, aber nicht so durcheinander. Warum struct metadata **m? Im Code wäre ein struct metadata *m wesentlich einfacher, denn Du änderst m ja nicht.

Wie gesagt funktioniert das ja auch, wenn ich *&m1 übergebe.

char* getMetaDataBezeichnung();
char* getMetaDataZusatzInfo();

Diese beiden Prototypen passen nicht zur Funktion weiter unten.

Stimmt. Leider ist dies trotzdem nicht die Ursache für den Fehler.

Ich kompiliere mit ...

$ gcc Metadata.c Main.c -o Main -std=c99

-Wall -pedantic sind oft hilfreich.

-Wall zeigt keinen Unterschied. Weder beim Kompilieren, noch beim Ausführen. Der Fehler ist derselbe.
-pendantic, oder ähnliches kennt der Compiler nicht.

markus@archy ~/C-Programme/Projekte/Dateibrowser $ ./Main
Main: malloc.c:3074: sYSMALLOc: Assertion `(old_top == (((mbinptr) (((char *) &((av)->bins[((1) - 1) * 2])) - __builtin_offsetof (struct malloc_chunk, fd)))) && old_size == 0) || ((unsigned long) (old_size) >= (unsigned long)((((__builtin_offsetof (struct malloc_chunk, fd_nextsize))+((2 * (sizeof(size_t))) - 1)) & ~((2 * (sizeof(size_t))) - 1))) && ((old_top)->size & 0x1) && ((unsigned long)old_end & pagemask) == 0)' failed.
Abgebrochen

Was hat das zu bedeuten?

Ein Paranoia-Test in malloc ist fehlgeschlagen. Du ruft offenbar malloc() mit Parametern auf, die dem malloc()-Code nicht gefallen.

Hmmm. Wie finde ich heraus, was malloc dabei nicht gefällt?

Ironischerweise funktioniert das Ganze aber, wenn ich die Funktion setMetaDataZusatzInfo kommentiere und somit nicht benutze. Wird also nur die erste Funktion aufgerufen funktioniert das Programm und ich bekomme die Ausgabe "Das ist die erste Bezeichnung". Kann sich das jemand erklären?

Verbogene Zeiger, Syntaxfehler, ...

Heißt das, dass es vorher immer nur zufälligerweise richtig funktioniert hat?

Fang mit -Wall -pedantic an.

Wie gesagt, keine Unterschiede, bzw. nicht vorhanden.

Meine zweite Frage betrifft die Pointerzuweisung. In metadata.h schreibe ich typedef struct metadata *MetaData; Ich kann nicht typedef struct metadata MetaData; schreiben, damit dieses teils objektorientierte Paradigma funktioniert (eigentlich wollte ich versuchen, die Struktur auf dem Stack zu erzeugen, aber das scheint irgendwie nicht zu funktionieren, da ja dann die Größe der Struktur nicht bekannt ist).

Doch, Du definierst die struct metadata nur an der falschen Stelle, im C-Code statt im Header.

Die Frage, wo denn nun was genau deklariert und/oder initialisiert werden sollte, stellte ich mir ohnehin anfangs.

Eine letzte Frage hätte ich noch zum Freigeben des Speichers. In Main führe ich ein free(m1) aus, was aber nicht ganz sauber wäre. Ich müsste die einzelnen Variablen der Struktur einzeln für sich auch freigeben, da ein killen der Struktur ja nicht heißt, dass auch die Variablen freigegeben wurden

Richtig. Wenn Du Dich nicht wie ein Assembler-Programmierer mit der Speicherverwaltung rumschlagen willst, nimm Ruby, Perl oder ähnliches. Ansonsten initialisiere deine frische struct metadata so, dass alle später allozierten Pointer erst einmal auf NULL gesetzt sind. Schreibe eine freeMetadata()-Methode, die alle Zeiger-Elemente der Struktur, die nicht NULL sind, wieder freigibt.

Ok, verstanden.

Markus

[*Markus]

Jetzt habe ich das nächste Problem. Füge ich folgende Funktion hinzu:

  
void destroyMetaData(struct metadata *m)   {  
	if (m->bezeichnung != NULL)  
		free(m->bezeichnung);  
	if (m->zusatzinfo != NULL)  
		free(m->zusatzinfo);  
		  
	free(m);  
}

und rufe die Funktion in Main so auf:

  
#include <stdio.h>  
#include <stdlib.h> 					//malloc  
#include "metadata.h"  
  
int main(void)   {  
  
    MetaData m1 = (MetaData)malloc(sizeof(MetaData));  
  
    setMetaDataBezeichnung(*&m1, "Das ist die erste Bezeichnung");  
    //setMetaDataZusatzInfo(*&m1, "Das ist eine zusätzliche Information");  
    printf("%s\n", getMetaDataBezeichnung(m1));  
    //printf("%s\n", getMetaDataZusatzInfo(m1));  
  
    destroyMetaData(m1);  
  
return 0;  
}  

...dann bekomme ich folgende Ausgabe:

markus@archy ~/C-Programme/Projekte/Dateibrowser $ ./Main
Das ist die erste Bezeichnung
*** glibc detected *** ./Main: free(): invalid next size (fast): 0x09e77018 ***
======= Backtrace: =========
/lib/libc.so.6[0xb7f429b1]
/lib/libc.so.6[0xb7f440b2]
/lib/libc.so.6(cfree+0x6d)[0xb7f4717d]
./Main[0x8048679]
./Main[0x80486e9]
/lib/libc.so.6(__libc_start_main+0xe6)[0xb7eeda36]
./Main[0x8048481]
======= Memory map: ========
08048000-08049000 r-xp 00000000 08:15 2454474    /home/markus/C-Programme/Projekte/Dateibrowser/Main
08049000-0804a000 rwxp 00000000 08:15 2454474    /home/markus/C-Programme/Projekte/Dateibrowser/Main
09e77000-09e98000 rwxp 00000000 00:00 0          [heap]
b7d00000-b7d21000 rwxp 00000000 00:00 0
b7d21000-b7e00000 ---p 00000000 00:00 0
b7ed6000-b7ed7000 rwxp 00000000 00:00 0
b7ed7000-b8017000 r-xp 00000000 08:13 647193     /lib/libc-2.10.1.so
b8017000-b8019000 r-xp 00140000 08:13 647193     /lib/libc-2.10.1.so
b8019000-b801a000 rwxp 00142000 08:13 647193     /lib/libc-2.10.1.so
b801a000-b801e000 rwxp 00000000 00:00 0
b8022000-b803f000 r-xp 00000000 08:13 231795     /usr/lib/libgcc_s.so.1
b803f000-b8040000 rwxp 0001c000 08:13 231795     /usr/lib/libgcc_s.so.1
b8040000-b8041000 rwxp 00000000 00:00 0
b8041000-b8042000 r-xp 00000000 00:00 0          [vdso]
b8042000-b805e000 r-xp 00000000 08:13 647192     /lib/ld-2.10.1.so
b805e000-b805f000 r-xp 0001b000 08:13 647192     /lib/ld-2.10.1.so
b805f000-b8060000 rwxp 0001c000 08:13 647192     /lib/ld-2.10.1.so
bfc20000-bfc35000 rw-p 00000000 00:00 0          [stack]
Abgebrochen

Also ällmählich verstehe ich gar nichts mehr. Wieso funktioniert das Freigeben des Speichers nicht? Es liegt definitiv an diesen Aufrufen:

if (m->bezeichnung != NULL)  
	free(m->bezeichnung);  
if (m->zusatzinfo != NULL)  
	free(m->zusatzinfo);  

...nur free(m) funktioniert.

Markus.

[Christoph]

Moin.

Hat sich an dem Quelltext im Vergleich zur ersten Nachricht etwas geändert? Falls ja, könntest du den Code nochmal komplett schicken?

Ansonsten zwei Bemerkungen: Der Ausdruck \*&m1 ist identisch mit m1, da der indirection-Operator \* gerade das Inverse des Adress-Operators & ist.

Außerdem ist es unnötig, vor dem Aufruf von free() auf NULL zu testen: Wird an free() ein Null-Zeiger übergeben, macht die Funktion einfach gar nichts.

Christoph

[*Markus]

Hallo Christoph,

Aktuell bin ich nicht an meinem Heimrechner um den Code posten zu können. Im Prinzip hat sich am Code nichts geändert. Ich habe in den Funktionen nur die Pointerbehandlung vereinfacht, indem ich die Funktionen so abänderte:

  
void setMetaDataBezeichnung(struct metadata *m, const char *s)  {  
         //Falls Zeichenkette länger als MAX_BEZ ist, nur MAX_BEZ+1 (+1 wg. \0) allozieren.  
         if (strlen(s) >= MAX_BEZ)  
            m->bezeichnung = (char*)calloc(MAX_BEZ+1,sizeof(char));  
         else  
                  m->bezeichnung = (char*)calloc(strlen(s)+1,sizeof(char));  
  
         if (m->bezeichnung == 0)  {  
                  printf("Fehler bei der Speicherallozierung");  
                  exit(1);  
         }  
         //Maximal MAX_BEZ Zeichen kopieren. \0 wird automatisch angehängt, da +1 noch frei ist.  
         strncpy(m->bezeichnung, s, MAX_BEZ);  
}  

...sowie ich die Funktion destroyMetaData hinzugefügt wurde (sowohl der Prototyp im Headerfile).
In Main.c wurde die Instanz der Struktur mit *&m übergeben. Ich werde am Abend deinen Tipp ausprobieren, sowie ich auch noch ausprobieren muss wie das Verhalten ist, wenn ich die Struktur im Headerfile deklariere. Sollte aber meiner Meinung nach keinen Unterschied machen, d.h. die Fehler werden wahrscheinlich immer noch auftreten.

Markus.

[Alexander (HH)]

Moin Moin!

Außerdem ist es unnötig, vor dem Aufruf von free() auf NULL zu testen: Wird an free() ein Null-Zeiger übergeben, macht die Funktion einfach gar nichts.

Bist Du Dir sicher, dass das in den C-Standards steht? Ich denke, das ist eher eine GNU-Erweiterung. Es schadet jedenfalls nicht, auf pointer!=NULL zu prüfen.

Alexander

[Christoph]

Moin.

Außerdem ist es unnötig, vor dem Aufruf von free() auf NULL zu testen
Bist Du Dir sicher, dass das in den C-Standards steht?

C89 Abschnitt 4.10.3.2 bzw. C99 Abschnitt 7.20.3.2, §2: "If ptr is a null pointer, no action occurs."

Christoph

[Christian Seiler]

Hallo,

MetaData m1 = (MetaData)malloc(sizeof(MetaData));

Ohne mir das jetzt genauer angesehen zu haben: Die Zeile ist garantiert falsch. Wenn Du etwas mit sizeof(MetaData) allozierst, dann gibt Dir malloc() einen Zeiger auf einen Speicherbereich zurück, der die Größe des Objekts MetaData hat - aber den kannst Du eigentlich nur in einen Zeiger auf MetaData casten und nicht auf MetaData selbst... Und wegen des Casts haut Dir das Dein Compiler natürlich nicht um die Ohren.

Korrekt wäre wohl irgendwie sowas wie:

MetaData *m1 = (MetaData *) malloc (sizeof (MetaData));

Und dann halt alles andere entsprechend ändern.

Viele Grüße,
Christian

[*Markus]

Hallo Christian,

MetaData m1 = (MetaData)malloc(sizeof(MetaData));

Ohne mir das jetzt genauer angesehen zu haben: Die Zeile ist garantiert falsch. Wenn Du etwas mit sizeof(MetaData) allozierst, dann gibt Dir malloc() einen Zeiger auf einen Speicherbereich zurück, der die Größe des Objekts MetaData hat - aber den kannst Du eigentlich nur in einen Zeiger auf MetaData casten und nicht auf MetaData selbst... Und wegen des Casts haut Dir das Dein Compiler natürlich nicht um die Ohren.

Korrekt wäre wohl irgendwie sowas wie:

MetaData *m1 = (MetaData *) malloc (sizeof (MetaData));

danke für den Tipp. Der Hinweis sieht schon mal vielversprechend aus. Ich freue mich darauf, es heute Abend auszuprobieren.

Viele Grüße,
Markus

[Christoph]

Moin.

MetaData m1 = (MetaData)malloc(sizeof(MetaData));
Korrekt wäre wohl irgendwie sowas wie:

MetaData *m1 = (MetaData *) malloc (sizeof (MetaData));

Nein, denn

typedef struct metadata *MetaData;

d.h. korrekt wären

MetaData m1 = malloc(sizeof(*m1));

oder

MetaData m1 = malloc(sizeof(struct metadata));

Hier sieht man, warum es nicht unbedingt eine gute Idee sein muss, den Variablentyp mit Hilfe von typedefs zu verschleiern.

Und noch eine Bemerkung: Das casting des Rückgabewerts von malloc() ist ein C++-Idiom: In C werden void \* implizit bei der Zuweisung konvertiert.

Christoph

[*Markus]

Hallo,

leider bringen sämtliche Kombinationen nicht das gewünschte Ergebnis. Das Programm stürzt weiter ab genauso wie vorher. Kommentiere ich in Main.c die zweiten setter und getter aus, wird zwar die erste Ausgabe angezeigt, doch dann bekomme ich wieder den Heap-Crash mit free(). Irgendwie ist das Ganze unberechenbar. Hier die aktuelle Version:

Main.c
-----------------------------------------

  
#include <stdio.h>  
#include <stdlib.h> 					//malloc  
#include "metadata.h"  
  
int main(void)   {  
  
    MetaData m1 = malloc(sizeof(struct metadata));  
    //m1 ist gleichbedeutend mit *&m1  
    setMetaDataBezeichnung(m1, "Das ist die erste Bezeichnung");  
    setMetaDataZusatzInfo(m1, "Das ist eine zusätzliche Information");  
    printf("%s\n", getMetaDataBezeichnung(m1));  
    printf("%s\n", getMetaDataZusatzInfo(m1));  
  
    destroyMetaData(m1);  
  
return 0;  
}  
  
metadata.h  
-----------------------------------------  
#ifndef METADATA_H  
#define METADATA_H  
  
#define MAX_INFO 100  
#define MAX_BEZ 50  
  
struct metadata  {  
	char *bezeichnung;  
	char *zusatzinfo;  
};  
  
typedef struct metadata *MetaData;  
  
void setMetaDataBezeichnung(struct metadata *m, const char *s);  
void setMetaDataZusatzInfo(struct metadata *m, const char *s);  
char* getMetaDataBezeichnung(struct metadata *m);  
char* getMetaDataZusatzInfo(struct metadata *m);  
void destroyMetaData(struct metadata *m);  
  
#endif  
  
  
Metadata.c  
-----------------------------------------  
#include <stdio.h>  
#include <string.h>			//strlen, strncpy  
#include <stdlib.h>			//calloc  
#include "metadata.h"  
  
void setMetaDataBezeichnung(struct metadata *m, const char *s)  {	  
	 //Falls Zeichenkette länger als MAX_BEZ ist, nur MAX_BEZ+1 (+1 wg. \0) allozieren.  
	 if (strlen(s) >= MAX_BEZ)  
	    m->bezeichnung = (char*)calloc(MAX_BEZ+1,sizeof(char));  
	 else  
	 	 m->bezeichnung = (char*)calloc(strlen(s)+1,sizeof(char));  
	 	  
	 if (m->bezeichnung == 0)  {  
	 	 printf("Fehler bei der Speicherallozierung");  
	 	 exit(1);  
	 }  
	 //Maximal MAX_BEZ Zeichen kopieren. \0 wird automatisch angehängt, da +1 noch frei ist.  
	 strncpy(m->bezeichnung, s, MAX_BEZ);  
}  
  
void setMetaDataZusatzInfo(struct metadata *m, const char *s)   {  
	 //Falls Zeichenkette länger als MAX_INFO ist, nur MAX_INFO+1 (+1 wg. \0) allozieren.  
	 if (strlen(s) >= MAX_INFO)  
	    m->zusatzinfo = (char*)calloc(MAX_INFO+1,sizeof(char));  
	 else  
	 	 m->zusatzinfo = (char*)calloc(strlen(s)+1,sizeof(char));  
	 	  
	 if (m->zusatzinfo == 0)  {  
	 	 printf("Fehler bei der Speicherallozierung");  
	 	 exit(1);  
	 }  
	 //Maximal MAX_INFO Zeichen kopieren. \0 wird automatisch angehängt, da +1 noch frei ist.  
	 strncpy(m->zusatzinfo, s, MAX_INFO);  
}  
  
char* getMetaDataBezeichnung(struct metadata *m)   {  
	 return m->bezeichnung;  
}  
  
char* getMetaDataZusatzInfo(struct metadata *m)   {  
	 return m->zusatzinfo;  
}  
  
void destroyMetaData(struct metadata *m)   {  
	if (m->bezeichnung != NULL)  
		free(m->bezeichnung);  
	if (m->zusatzinfo != NULL)  
		free(m->zusatzinfo);  
		  
	free(m);  
}  
  

Danke,
Markus

[Christoph]

Moin.

Hier stecken zwei Fehler:

  
    strncpy(m->bezeichnung, s, MAX_BEZ);  
    strncpy(m->zusatzinfo, s, MAX_INFO);  

strncpy() schreibt *immer* soviele Zeichen wie angegeben in den Ziel-buffer: ist der Quell-string zu kurz, wird mit Nullen aufgefüllt.

Da du aber falls die string-Längen kleiner als die Maximallängen sind weniger Speicher allozierst, erzeugt der Aufruf einen buffer overflow.

Mit folgenden Funktionen läuft das Programm bei mir problemlos durch:

  
#include <assert.h>  
  
void setMetaDataBezeichnung(struct metadata *m, const char *s)  
{  
    size_t len = strlen(s);  
    if(len > MAX_BEZ) len = MAX_BEZ;  
  
    m->bezeichnung = malloc(len + 1);  
    assert(m->bezeichnung);  
  
    memcpy(m->bezeichnung, s, len);  
    m->bezeichnung[len] = 0;  
}  
  
void setMetaDataZusatzInfo(struct metadata *m, const char *s)  
{  
    size_t len = strlen(s);  
    if(len > MAX_INFO) len = MAX_INFO;  
  
    m->zusatzinfo = malloc(len + 1);  
    assert(m->zusatzinfo);  
  
    memcpy(m->zusatzinfo, s, len);  
    m->zusatzinfo[len] = 0;  
}  

Christoph

[*Markus]

Hallo,

vielen Dank. Es klappt nun (auch mit dem free()). Habe probehalber mal die If-Verzweigung auskommentiert, um immer die volle Länge an Speicher zu reservieren. Dann entstehen keine Programmabstürze mehr.

Danke,
Markus

[Alexander (HH)]

Moin Moin!

Jetzt habe ich das nächste Problem.
...dann bekomme ich folgende Ausgabe:

markus@archy ~/C-Programme/Projekte/Dateibrowser $ ./Main
Das ist die erste Bezeichnung
*** glibc detected *** ./Main: free(): invalid next size (fast): 0x09e77018 ***

Du hast Dir den Heap mittlerweile so gründlich zerschossen, dass die glibc in Panik verfällt. Du machst also offensichtlich Dinge mit Pointern, die Du besser nicht machen solltest. Christians Hinweis ist schonmal ein erster Schritt.

Übrigens: gcc kennt -pedantic seit mindestens fünf, eher 10 Jahren, aber -pendantic ist Quatsch.

Alexander

[Andreas Pflug]

Wenn ich das richtig sehe, hast Du einen Strukturtyp
unter dem Namen

struct metadata

definiert und dann noch den Typ

MetaData

als Zeiger auf diesen Strukturtyp.
In Main.c definierst Du nun

  
    MetaData m1 = (MetaData)malloc(sizeof(MetaData));  

Wahrscheinlich wolltest Du hier einen Pointer m1 definieren,
der auf einen Speicherbereich zeigt, in dem sich
eine "metadata"-Struktur breitgemachen kann.
Das Problem dabei ist, dass sizeof(MetaData) hier nur
die Größe eines einzelnen Pointers wiedergibt, d. h. je nach
Betriebssystem 4 oder 8 byte. Was Du wolltest, ist
wahrscheinlich sizeof(metadata)?

Deine Struktur besteht ja aus zwei Pointern, daher
kann es sein, dass Dein Programm beim ersten
Funktions-Aufruf (d. h. Zugriff auf den ersten Pointer)
"zufällig" noch funktioniert und erst beim Setzen
des zweiten Pointers, der dann außerhalb des allozierten
Bereichs liegt, aussteigt.

Abgesehen davon würde ich versuchen, den Code etwas
zu entrümpeln. Es macht IMHO keinen Sinn, den
Funktionen setMetaDataBezeichnung(...) als Argument
einen Typ Pointer-auf-Pointer-auf-Struktur zu übergeben,
Pointer-auf-Struktur sollte völlig ausreichend sein.

MfG

Andreas

[*Markus]

Hallo,

danke für den Tipp, aber selbst MetaData m1 = malloc(sizeof(struct metadata)); führt zu den selben Programmabbrüchen.

Allmählich scheinen nun die Optionen auszugehen.

Viele Grüße,
Markus

[Andreas Pflug]

Hallo,

danke für den Tipp, aber selbst MetaData m1 = malloc(sizeof(struct metadata)); führt zu den selben Programmabbrüchen.

Wie gesagt, scheint mir der Code insgesamt recht unübersichtlich,
vor allem wegen den verschachtelten Typdefinitionen
metadata und MetaData. Als Alternative würde ich
mal folgenden Code in den Raum stellen (der Einfachheit
halber alles in ein File, d. h. ohne Headerfiles):

  
#include<stdio.h>  
#include<string.h>  
#include<stdlib.h>  
#include<malloc.h>  
  
#define MAX 25  
  
struct metadata {  
  char *bezeichnung;  
  char *zusatzinfo;  
};  
  
void set_str(char **c, const char *s) {  
  int len = strlen(s);  
  if (len>MAX) len=MAX;  
  *c = realloc(*c, (len+1)*sizeof(char));  
  if(!*c) { puts("Kein Speicher mehr(?!)\n"); exit(1); }  
  strncpy(*c, s, len);  
}  
  
void set_bez(struct metadata *m, const char *s) { set_str(&m->bezeichnung, s); }  
void set_zus(struct metadata *m, const char *s) { set_str(&m->zusatzinfo, s); }  
  
void kill_metadata(struct metadata *m) {  
  free(m->bezeichnung);  
  free(m->zusatzinfo);  
}  
  
int main(void) {  
  struct metadata m = {NULL, NULL};  
  set_bez(&m, "Erste Zeile");  
  set_zus(&m, "Die zweite Zeile ist viel zu lang");  
  
  printf("m.bezeichnung = %s\n"  
         "m.zusatzinfo  = %s\n", m.bezeichnung, m.zusatzinfo);  
  
 kill_metadata(&m);  
 return 0;  
}  

Das sollte in etwa das tun, was ich aus Deinem Code
herauszulesen glaube...

MfG

Andreas

[Christoph]

Moin.

Sehr schön: das Speicherleck bei Zuweisung neuer Werte von bezeichnung und zusatzinfo war mir entgangen, was du dank realloc() vermeidest:

  
 void set_str(char **c, const char *s) {  
   int len = strlen(s);  
   if (len>MAX) len=MAX;  
   *c = realloc(*c, (len+1)*sizeof(char));  
   if(!*c) { puts("Kein Speicher mehr(?!)\n"); exit(1); }  
   strncpy(*c, s, len);  
 }  

Trotzdem steckt hier ein Bug: falls strlen(s) == MAX, wird das Null-Endzeichen von s nicht mitkopiert und *c ist nur dann korrekt terminiert, falls zufälligerweise (*c)[len] == 0.

Statt strncpy(*c, s, len) sollte besser

  
    memcpy(*c, s, len);  
    (*c)[len] = 0;  

verwendet werden.

Außerdem ist es in Objekt-orientiertem C üblich, wie in Markus' ursprünglichem Code mit Zeigern auf die Strukturen zu arbeiten. Dabei wird der Code zur Speicherallokation und Initialisierung ausgelagert, was robuster ist: vergisst der Programmierer bei deiner Variante die Initialisierung mit Null-Zeigern, schlägt set_bez() und set_zus() fehl, da realloc() dann ungültige Zeiger übergeben werden.

Ich würde daher folgendes Vorschlagen:

  
void kill_metadata(struct metadata *m)  
{  
    free(m->bezeichnung);  
    free(m->zusatzinfo);  
    free(m);  
}  
  
struct metadata *create_metadata(void)  
{  
    struct metadata *m = malloc(sizeof(*m));  
    if(!m) return NULL;  
  
    m->bezeichnung = NULL;  
    m->zusatzinfo = NULL;  
    return m;  
}  
  
int main(void)  
{  
    struct metadata *m = create_metadata();  
    assert(m);  
  
    set_bez(m, "Erste Zeile");  
    set_zus(m, "Die zweite Zeile ist viel zu lang");  
  
    printf("m->bezeichnung = %s\n"  
           "m->zusatzinfo  = %s\n", m->bezeichnung, m->zusatzinfo);  
  
    kill_metadata(m);  
}  

Christoph

[Andreas Pflug]

Hallo,

Trotzdem steckt hier ein Bug: falls strlen(s) == MAX, wird das Null-Endzeichen von s nicht mitkopiert und *c ist nur dann korrekt terminiert, falls zufälligerweise (*c)[len] == 0.

Statt strncpy(*c, s, len) sollte besser

memcpy(*c, s, len);
    (*c)[len] = 0;

> verwendet werden.  
  
Stimmt und danke für Deinen Hinweis!  
Das Problem tritt auch bereits dann auf, wenn  
man erst einen längeren und danach einen kürzeren  
String an die selbe Stelle schreibt. Man kann  
dem allerdings schon abhelfen, indem man  
  
`strncpy(*c, s, len+1)`{:.language-c}  
  
schreibt, da ich ja ohnehin len+1 bytes alloziere und  
bei char \*s davon ausgegangen werden kann,  
dass dieser String mit einem Null-Charakter endet (falls  
nicht, dann ...)).  
  

> void kill\_metadata(struct metadata \*m)  
> {  
>     free(m->bezeichnung);  
>     free(m->zusatzinfo);  
>     free(m);  
> }  
  
Damit würde meine main-Routine beim Beenden  
eine "double free or corruption"-Meldung oder ähnliches spucken,  
mit dem von Dir geschilderten Konzept  
ist das natürlich konsistent.  
Programmieren in "objektorientiertem C" ist allerdings etwas, das  
ich mir nicht im Übermaß antun will ;-).  
Vielleicht findet sich noch jemand, der eine  
Version des Codes in State-of-the-Art-C++ postet?  
  
MfG  
  
Andreas

[Christian Seiler]

Hallo Andreas,

Vielleicht findet sich noch jemand, der eine
Version des Codes in State-of-the-Art-C++ postet?

Deine Variante nach C++ portiert:

#include <iostream>  
#include <string>  
  
class Metadata {  
  public:  
    // Default-Konstruktor  
    Metadata()  
      : bezeichnung(), zusatzinfo()  
    {  
    }  
  
    // Konstruktor mit Initialisierung (für std::string)  
    Metadata(const std::string& bezeichnung, const std::string& zusatzinfo)  
      : bezeichnung(bezeichnung), zusatzinfo(zusatzinfo)  
    {  
    }  
  
    // Konstruktor mit Initialisierung (für const char *)  
    Metadata(const char *bezeichnung, const char *zusatzinfo)  
      : bezeichnung(bezeichnung), zusatzinfo(zusatzinfo)  
    {  
    }  
  
    // Konstruktor mit Initialisierung (gemischt)  
    Metadata(const std::string& bezeichnung, const char *zusatzinfo)  
      : bezeichnung(bezeichnung), zusatzinfo(zusatzinfo)  
    {  
    }  
  
    // Konstruktor mit Initialisierung (gemischt)  
    Metadata(const char *bezeichnung, const std::string& zusatzinfo)  
      : bezeichnung(bezeichnung), zusatzinfo(zusatzinfo)  
    {  
    }  
  
    // Copy-Konstruktor  
    Metadata(const Metadata& o)  
      : bezeichnung(o.bezeichnung), zusatzinfo(o.zusatzinfo)  
    {  
    }  
  
    // Zuweisungsoperator  
    Metadata& operator=(const Metadata& o)  
    {  
      // self-assign ist kein Problem in diesem Fall  
      bezeichnung = o.bezeichnung;  
      zusatzinfo = o.zusatzinfo;  
      return *this;  
    }  
  
    // Destruktor hier nicht benötigt  
  
    // Felder (können hier Public sein)  
    std::string bezeichnung;  
    std::string zusatzinfo;  
};  
  
int main (int argc, char **argv)  
{  
  Metadata m("Erste Zeile", "Die zweite Zeile ist viel zu lang");  
  std::cout << "m.bezeichnung = " << m.bezeichnung << '\n'  
            << "m.zusatzinfo  = " << m.zusatzinfo  << std::endl;  
  m.bezeichnung = "Erste Zeile (modifiziert)";  
  std::cout << "m.bezeichnung = " << m.bezeichnung << '\n'  
            << "m.zusatzinfo  = " << m.zusatzinfo  << std::endl;  
  
  // oder auf dem Heap  
  Metadata *m2 = new Metadata ("Dritte Zeile", "Die vierte Zeile ist viel zu lang");  
  std::cout << "m2->bezeichnung = " << m2->bezeichnung << '\n'  
            << "m2->zusatzinfo  = " << m2->zusatzinfo  << std::endl;  
  delete m2;  
  
  return 0;  
}

Viele Grüße,
Christian