[C] malloc/realloc

[Verwijderd]

Ik probeer mails uit het mailbestand te halen en deze vervolgens in structs te zetten en wegschrijven in een tijdelijk bestand om er later andere bewerkingen mee te doen. Maar ik krijg weer de welbekende segmentation faults. Dit is mijn code:

code:

#include <stdio.h>
#include <ctype.h>
#include <string.h>
#include <iostream>
#include <sys/types.h>
#include <stdlib.h>
//#include <regexx.hh>

//using namespace regexx;

struct Messages
{
      int flag;
      char *mail;
};

FILE *tmpf;

void getmaildrop (char *user)
{
      char loc [32];
      //Regexx rxx;
      FILE *mailfile;

      sprintf (loc, "/var/spool/mail/%s", user); // Locatie bepalen
      if ((mailfile = fopen (loc, "r")) == NULL) // Openen
      {
            printf ("ERROR OPENING MAILFILE\n");
      }
      else
      {
            struct Messages *entry;
            char buf [1024];
            char *mail;

            tmpf = tmpfile (); // Temporary file aanmaken
            entry = (struct Messages *D malloc (sizeof (int) + 1024 * sizeof (char));
            mail = (char *D malloc (1024 * sizeof (char)); // Geheugen alloceren

            entry->mail = (char *D malloc (1); // Geheugen alloceren
            entry->mail = "\0"; // En vullen

printf ("Init gedaan...\n");

            while (!feof (mailfile)) // Doorgaan tot End Of File (EOF)
            {
                fgets (buf, 1024, mailfile); // 1024 bytes inlezen

                if (/*rxx.exec (str, "From .+@.+")*/strstr (buf, "From ")) // Checken op nieuwe mails en EOF
                {
                      int newmail = 0;
                      sprintf (mail, "%s", buf); // Regel toevoegen

printf ("Nieuwe mail...\n");
                      while (!feof (mailfile) || newmail == 1)
                      {
                            fgets (buf, 1024, mailfile);
                            if (/*!rxx.exec (str, "From .+@.+")*/!strstr (buf, "From "))
                            {
printf ("Data wegschrijven in var...\n");
                                mail = (char *D realloc (mail, strlen (mail) + (1024 * sizeof (char))); // Geheugen vergroten met 1024 bytes
                                sprintf (mail, "%s%s", mail, buf);
                            }
                            else
                            {
                                long int fp;
                                newmail = 1;

printf ("Nieuwe mail wegschrijven naar temp...\n");

                                // Rekening houden met file pointer, terug plaatsen naar de juiste positie om data loss tegen te gaan
                                fp = ftell (mailfile);
                                fp = fp - strlen (buf);
                                fseek (mailfile, fp, SEEK_CUR);

printf ("File pointer goed gezet...\n");
                                // Nieuwe mail opgehaald, klaar voor wegschrijven
                                entry = (struct Messages *D realloc (entry, sizeof (struct Messages));
printf ("Opnieuw gealloceerd...\n");
printf ("Mail = %d\n", strlen (mail));
printf ("entry.mail = %d\n", strlen (entry->mail));
/***********************/
/* HIER GAAT HET FOUT  */
/***********************/
                                entry->mail = (char *D realloc (&entry->mail, strlen (mail)); // Geheugen aanpassen aan grootte mail
printf ("Opnieuw gealloceerd...\n");
                                entry->mail = strdup (mail); // Struct vullen
                                entry->flag = 0; // Vlag op 0 zetten
printf ("Struct gevuld...\n");

printf ("Klaar om weggeschreven te worden...\n");
                                fwrite (&entry, sizeof (entry), 1, tmpf); // Wegschrijven naar bestand
printf ("Mail weggeschreven naar temp...\n");
                            }
                      }

                }
            }
            free (mail); // Geheugen vrijgeven tegen memory leaks
            free (entry->mail);
            free (entry);
            fclose (mailfile); // Bestand sluiten
      }
}

main()
{
      getmaildrop ("root"); // Ff om te testen
      return 0;
}

Dat regexx gedoe dat gebruik ik om regexpressies uit te voeren, dat klopt wel en hoeft niet verandert te worden. Hoe ik dat realloc ook aanroep, ik krijg altijd een segmentation fault.

[Soultaker]

Euh, ik heb geen zin om smileys te decoderen naar C code. Zet er even [code] (dus met vierkante haken omheen) en zorg ervoor dat er goed ingesprongen wordt, dan wil ik er wel naar kijken.

Ok, je bent me voor

[Verwijderd]

Gebruikte per ongeluk < ipv [. Heb het al veranderd...

[Orphix]

Je laat entry naar een leeg geheugenblok wijzen. Vervolgens roep je entry->mail aan. Deze wijst dus naar 0 undefined -> segmentation fault.

_{brrr wat een taaltje}

[eek]

The strdup() function allocates sufficient memory for a copy of the string str, does the copy, and returns a pointer to it. The pointer may subsequently be used as an argument to the function free(3).

tijdje geleden dus manpage er maar ff bijgepakt, volgens
mij hoeft die hele realoc niet, of je doet puur een copy
ipv strdup.

[Verwijderd]

Als ik het dus goed begrijp, er onstaat dus een segmentation fault doordat entry->mail niet is gevuld? Want entry->mail is wel degelijk in het begin gealloceerd (zie regel 40).

Die strdup moet mail kopieren. Moet ik ff veranderen in strcpy of zoiets dergelijks. Manpage van strdup heb ik 8 weken geleden gelezen...

[windancer]

entry->mail is al een pointer, waarom geef je dan &entry->mail aan realloc mee ? realloc heeft echt alleen maar een pointer nodig, geen pointer naar een pointer.

[Soultaker]

code:

entry = (struct Messages * realloc (entry, sizeof (struct Messages));

Nu alloceer je dus een nieuwe structoftewel (in de praktijk) 8 bytes geheugen, waar precies een getal (flag) en een pointer (mail) in passen. Die gegevens zijn op geen enkele manier geinitialiseerd dus hier:

code:

printf ("entry.mail = %d\n", strlen (entry->mail));

Derefereer je een willekeurige pointerwaarde. Dat gaat natuurlijk nooit goed!

Wat je vervolgens probeert te doen is me een raadsel:

code:

entry->mail = (char *D realloc (&entry->mail, strlen (mail));

entry->mail is niet geinitialiseerd, dus kan 'ie in principe ook niet ge-realloc'ed worden (eventueel zou dat wel kunnen als 'ie op NULL was geinitialiseerd, maar dan was het effect hetzelfde als een malloc).

Sowieso reserveer je nu geen ruimte voor het afsluitende 0-karakter.

Het is ook zinloos om deze code te hebben, als je al (terecht) deze volgende regel gebruikt:

code:

entry->mail = strdup (mail); // Struct vullen

strdup() allocceert al geheugen voor mail; er is dus geen reden om deze eerst zelf te alloceren.

Ten slotte een suggestie: als je records alleen maar één keer wilt gebruiken, kun je gewoon elke keer hun inhoud overschrijven. Er is zelden een reden om een struct (die normaliter een vaste grootte heeft) te reallocen.

De code wordt dan zoiets:

code:

struct Messages entry;
while( .. )
{
  ..
  entry->mail = strdup (mail);
  entry->flag = 12345;
  ..
  /* Doet iets met entry */
  ..
  /* Door strdup gealloceerde geheugen opruimen */
  free(entry->mail); 
}

Jou code is echter op alle fronten fout, want je schrijft een struct met daarin een pointer weg naar het bestand, wat zinloos is. De struct bevat dan bijvoorbeeld '0 (flag) 473492 (pointerwaarde)'. Per write()-call worden er dus precies 8 bytes weggeschreven. De laatste 4 zijn alleen zinnig binnen dit proces en de daadwerkelijke string ('mail') wordt nergens opgeslagen.

Ik geloof dat ik wel een compleet boek kan schrijven over de fouten die in deze code ten toon worden gespreid, dus ik stel voor dat je eerst nog even een serieuze poging waagt C te leren voordat je aan een daadwerkelijke applicatie begint.

Als oefening is het natuurlijk niet verkeerd.

In ieder geval een paar tips:
1. Gebruik realloc uitsluitend bij datatypen van variabele grootte (strings en arrays bijvoorbeeld).
2. Initialiseer alle pointer-waarden (als je ze niet direct gaat toekennen) altijd op NULL.
3. Zorg ervoor dat je kan garanderen dat tegenover elke aanroep van malloc precies één aanroep van free staat. Het is good practice om een vrijgegeven pointerwaarde ook weer op NULL te zetten.

[Verwijderd]

Reply op winddancer:
Dat stond nog in de code die ik meestuurde, heb al gezegd dat ik van alles heb geprobeerd. &entry->mail was namelijk eerst entry->mail.

Reply op Soultaker:
Een mail wordt ingelezen als een hele lange string. Elke mail is anders qua grootte. Mij lijkt dan dat het het beste is om malloc/realloc te gebruiken.

C is niet mijn sterkste kant. Ik heb ook nooit een geheel boek gelezen, alleen de reader die je bij de lessen kreeg, er was ook geen boek vereist of aanbevolen. Om het nog erger te maken: tijdens die lessen moesten we een kernel module schrijven...:( Toch maar een klacht indienen bij de school...

[Soultaker]

Op zondag 30 juni 2002 16:19 schreef Dominique het volgende:
Een mail wordt ingelezen als een hele lange string. Elke mail is anders qua grootte. Mij lijkt dan dat het het beste is om malloc/realloc te gebruiken.

Uiteraard, maar dan wel op de dynamische string die die mail bevat en niet de statische struct, die slechts de flags en een pointer bevat.

C is niet mijn sterkste kant. Ik heb ook nooit een geheel boek gelezen, alleen de reader die je bij de lessen kreeg, er was ook geen boek vereist of aanbevolen. Om het nog erger te maken: tijdens die lessen moesten we een kernel module schrijven...:( Toch maar een klacht indienen bij de school...

Je bent met redelijk ingewikkelde dingen bezig, dus kan het misschien geen kwaad je toch wat meer in de details (?) te verdiepen. Op internet is genoeg te vinden en anders heeft de plaatselijke (computer)boekhandel vast ook wel iets.

[Verwijderd]

Ben weer bezig geweest, weer de hele boel veranderd. Dit maal zonder malloc/realloc.

Zo ziet mijn code er nu uit:

code:

#include <stdlib.h>
#include <stdio.h>
#include <sys/types.h>
#include <string.h>

FILE *tmpf;

struct Messages
{
      int flag;
      char *mail;
};

void getmaildrop (char *user)
{
      char loc [32];
      FILE *mailfile;

      sprintf (loc, "/var/spool/mail/%s", user);
      if ((mailfile = fopen (loc, "r")) == NULL) // Openen
      {
            printf ("ERROR OPENING MAILFILE\n");
      }
      else
      {
            struct Messages *entry;
            char rdbuf [1024];
            char *mail;

            mail = "\0";
            entry->mail = "\0";
            tmpf = tmpfile ();

            while (!feof (mailfile))
            {
                fgets (rdbuf, 1024, mailfile);

                if (strstr (rdbuf, "From root "))
                { // Mail begin gevonden
                      int newmail = 0;

                      while (!feof (mailfile) || newmail == 1)
                      {
                            fgets (rdbuf, 1024, mailfile);
                            if (!strstr (rdbuf, "From root "))
                            {
                                int length = strlen (mail) + strlen (rdbuf);
                                char tmpstr [length];

                                strcpy (tmpstr, mail);
                                strcat (tmpstr, rdbuf);

                                mail = strdup (tmpstr);
                            }
                            else
                            {

                                long int fp;

                                // Rekening houden met file pointer, terug plaatsen naar de juiste positie om data loss tegen te gaan
                                fp = ftell (mailfile);
                                fp = fp - strlen (rdbuf);
                                fseek (mailfile, fp, SEEK_CUR);

                                entry->mail = strdup (mail);
                                entry->flag = newmail;

                                fwrite (entry, sizeof (entry), 1, tmpf); // Wegschrijven naar bestand
                                newmail = 1;
                            }
                      }
                }
            }
      }
      fclose (mailfile);
}

int main ()
{
      getmaildrop ("root");
      return 0;
}

Dit maal gaat het fout bij het wegschrijven. Krijg segmentation fault. Vind ik toch vreemd gezien het feit dat die regel niet door mij is gemaakt, maar overgeschreven is uit een boek...

Ik ga morgen wel ff kijken voor een goed boek over C. De docent zei dat Unix network programming of zoiets wel goed was, alleen het prijskaartje is extreem...

[johnwoo]

code:

(...)

struct Messages *entry;

(...)

entry->mail = "\0";

(...)

Je declareert een pointer entry naar een struct Messages. De compiler alloceert hier maar 4 bytes aan geheugen voor je, precies genoeg voor die pointer. Voordat je die struct ook daadwerkelijk kan gaan gebruiken moet je er wat geheugen voor reserveren. In C++ doe je dat met new; in C zul je het zelf moeten allocaten en je pointer naar het begin van het gealloceerde stukje moeten zetten. Nu zit je iig met een pointer te werken die naar een stukje garbage wijst.

Je opties zijn dus:
- Als je met een pointer wil blijven werken, moet je die struct eerst even instantieren.
- Een stuk voor de hand liggender, vooral omdat je -zo te zien- die variabele entry niet buiten deze functie gebruikt: declareer gewoon een statische struct. Dus struct Messages *entry; wordt struct Messages entry;, en verder access je de members met de . operator ipv de -> operator. De compiler regelt verder het benodigde geheugen voor je struct.