free char* innerhalb von struct

nero08

Hallo!

Folgender Code:

Ich erstelle das struct wie folgt:

KV_Lexer* kvlex_create(const uint8_t *input, size_t length)
{
 struct kv_lexer *lex = calloc(1,sizeof(struct kv_lexer));
 if(lex != NULL)
 {
  lex->input = calloc(length+1,sizeof (const uint8_t));

  if(lex->buf != NULL)
  {
    if(input == NULL)
    {
      if(length != 0)
      {
        return NULL;
      }
    }
    else
    {

      strcpy((char*)lex->input, (const char*)input);
    }
    lex->offset = length; 
    return lex;
  }
  else
  {
    free(lex);
  }
 }

  return NULL;
}

Freigegeben wird der Speicher mit:

void kvlex_destroy(KV_Lexer *lex)
{
  if(lex != NULL)
    free(lex->input);

  free(lex);
}

Es wird einfach nur ein struct mit "123" erstellt und dann wieder detroyed.

Leider bekomme ich mit valgrind folgenden Fehler:
Invalid free() / delete / delete[] / realloc()
==10634== at 0x402B3D8: free (in /usr/lib/valgrind/vgpreload_memcheck-x86-linux.so)
==10634== by 0x807F016: free (testallocs.c:116)
==10634== by 0x8077595: kvlexer_destroy (lexer.c:73)
==10634== by 0x806739D: test_kvlex_scan_int_dec_simple_do (lexer.c:100)
==10634== by 0x8058DEF: test_kvlex_scan_int_dec_simple (lexer.c:89)
==10634== by 0x404C45B: ??? (in /usr/lib/libcunit.so.1.0.1)
==10634== by 0x404CEA4: CU_run_test (in /usr/lib/libcunit.so.1.0.1)
==10634== by 0x4051BC1: CU_console_run_tests (in /usr/lib/libcunit.so.1.0.1)
==10634== by 0x807E9EA: RunTests (testrunner.c:239)
==10634== by 0x807E721: TestMain (testrunner.c:276)
==10634== by 0x804915A: main (c2tests.c:116)
==10634== Address 0x8081e91 is not stack'd, malloc'd or (recently) free'd
==10634==

Das Problem tritt also bei: free(lex->input); auf

Ich kann mir nicht erklären, was ich falsch mache, und wieso das free in diesem Fall nicht funktionieren sollte.

achja,von der Korrektheit her macht obiges das richtige...

lg

simbad

Sieht irgendwie korrekt aus.
Wenn du aber KV_lexer global angelegt hast, dann kann das auch schief gehen. Also bei so einem Konstrukt

KV_Lexer l;

kvlex_destroy(&l);

Aber sowas machst du natürlich nicht.
Oder du hast einen nicht initialisierten Pointer.
Oder aber du zerschiesst dir irgendwo den Pointer auf die Struktur.

Vielmehr bleibt nicht.

Mich wundert etwas, dass Dein Code kvlex_destroy() beinhaltet, aber in der Valgrind Ausgabe kvlexer_destroy() steht.

nero08

simbad schrieb:

Sieht irgendwie korrekt aus.
Wenn du aber KV_lexer global angelegt hast, dann kann das auch schief gehen. Also bei so einem Konstrukt
KV_Lexer l;

kvlex_destroy(&l);
Aber sowas machst du natürlich nicht.
Oder du hast einen nicht initialisierten Pointer.
Oder aber du zerschiesst dir irgendwo den Pointer auf die Struktur.

Vielmehr bleibt nicht.

jetzt wo du es sagst. lex->input wird verändert, sodass input ein leer string ist. aber wieso hat free mit "" Probleme?

@Furble Wurble hab ihn ein wenig umgeschrieben fürs forum und das übersehen ^^

lg

nero08 schrieb:

jetzt wo du es sagst. lex->input wird verändert, sodass input ein leer string ist. aber wieso hat free mit "" Probleme?

Wie "verändert"?

"Verändert" im Sinne von

void f(KV_Lexer* l){
  l->input="";
}

Das wäre natürlich 'n Knaller...(Wie passend zu Silvester!)

nero08

Furble Wurble schrieb:

nero08 schrieb:

jetzt wo du es sagst. lex->input wird verändert, sodass input ein leer string ist. aber wieso hat free mit "" Probleme?

Wie "verändert"?

"Verändert" im Sinne von
void f(KV_Lexer* l){
  l->input="";
}
Das wäre natürlich 'n Knaller...(Wie passend zu Silvester!)

naja ich lese der Reihe nach Zeichen aus, am ende ist input dann leer. am ende bleibt, dann sowas wie du sagst übrig, was wäre die alternative?

Bin den Fehler durch umschreiben losgeworden.

Wutz

nero08 schrieb:

aber wieso hat free mit "" Probleme?

free hat keine Probleme.
Du bist der Problemfall.
Du bist nicht in der Lage, hier ein simples Copy+Paste deines Codes abzuliefern.
Du hast enorme Schwierigkeiten im Verständnis bei einfachsten C Sachverhalten wie char und Zeichenketten (ganz zu schweigen bei Zeiger+Array) wie auch deine anderen Endlos-Fragereien zeigen.
Suche dir ein andereres Hobby, aber nicht C Programmierung.
Ach ja, deine vorgeblichen free-Probleme rühren daher, dass du den zuvor mit calloc gesetzten Zeiger selbst änderst (und nicht die Daten, auf die der Zeiger verweist) und anschließend free diesen veränderten Zeiger übergibst.
Sei froh, dass free dabei abstürzt und dich somit auf deinen Fehler hinweist, sonst würdest du (wie alle Anfänger hier) davon ausgehen, dass dein Programm "funktioniert" (weil es (mit ignorierten Warnungen) compiliert und bei einem (idealisierten) Testlauf "funktioniert").

supertux

ich finde, es gibt noch ein Paar grundlegende Design Probleme hier.

lex->input = calloc(length+1,sizeof (const uint8_t));

wozu length + 1 ? Das macht nur bei Strings Sinn. Am besten wäre es die Größe im Struct mitzuspeichern

2. Zeile 8: lex->buf ist danke man: calloc bereits NULL . Die Überprüfung ist nicht falsch aber hier unnötig und macht den Code weniger lesbar.

strcpy((char*)lex->input, (const char*)input);

Das hier finde ich häßlich, grausam und zeigt, dass du nicht verstanden hast, was du da tust. input ist ein Array von unsigned 8-bit ints, kein String. Was ist wenn input auf [10, 13, 33, 0, 15, 25] zeigt? Nur ein Bruchteil des Array wird kopiert, weil strcpy ein 0 sieht und endet.

Was ist wenn input auf [10, 13, 33, 15, 25] zeigt? Dann hast du einen Bufferoverflow. Du hast ein undefiniertes Verhalten, denn man: strcpy wird nicht aufhören bis 0 in input gefunden wird und viel zu viele Daten kopieren. Du könntest auch Daten, die auf dem Heap sind, überschreiben... mit viel Glück stürtzt dein Program mit segfault ab, mit wenig Glück hast du etwas ähnliches wie bei Heartbleed, wo du mehr liest und speicherst als gewünscht ist.

Geh mal hier man: strcpy. Was steht da?

#include <string.h>

char *strcpy(char *restrict s1, const char *restrict s2);
The strcpy() function shall copy the STRING pointed to by s2 (including the terminating null byte) into the array pointed to by s1. If copying takes place between objects that overlap, the behavior is undefined.

ist das so schwer zu verstehen, dass strcpy nur mit wohl definierten Strings zu verwenden ist? Ansonsten musst du hier man: memcpy bzw. man: memmove (falls die Bereiche sich überlappen) verwenden

memcpy(lex->input, input, length * sizeof *input);

und deshalb sollest du length in struct speichern. Keine Ahnung ob du lex->offset dafür nutzt.