getc() segmentation fault

[anycolouryoulike]

Codice: Seleziona tutto

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

#define MAX 4

main(int argc, char *argv[])
{
  FILE *f;
  char *s;
  char *v[MAX - 1];
  f = fopen(argv[1], "r");
  s = malloc(2000);
  char *p = s;
  int i, c;
  c = getc(f);
  char newline, comment, word;
  newline = comment = word = i = 0;
  while (c = getc(f))
    {
      if (c == '#')
	{
	  if (newline)
	    comment = 1;
	  continue;
	}
      if (newline = (c == '\n'))
	{
	  if (comment)
	    comment = 0;
	  if (word)
	    {
	      word = 0;
	      *p++ = '\0';
	      if (i == MAX)
		break;
	    }
	  continue;
	}
      if (comment)
	continue;
      if (isspace(c))
	{
	  if (word)
	    {
	      word = 0;
	      *p++ = '\0';
	      if (i == MAX)
		break;
	    }
	  continue;
	}
      if (isalnum(c))
	{
	  if (! word)
	    {
	      v[++i -1] = p;
	      word = 1;
	    }
	  *p++ = c;
	}        
    }
  for (i = 0; i < MAX; i++)
    puts(v[i]);
}

Sto cercando di fare un piccolo programmino che memorizza MAX parole separate da spazi, saltando i commenti.

Codice: Seleziona tutto

$ cat prova
fadfa







# fadfadfadfadfada
#









 ahdahr

 gadg    dagad
$ a.out prova
zsh: segmentation fault  a.out prova

Con gdb ho visto che il segfault è generato da un ennesimo while (c = getc(f)).
Com'è possibile che getc() faccia terminare il programma?
Grazie

[Mario Vanoni]

getc(f)

f e` dichiarato FILE *f

[anycolouryoulike]

getc(f)

f e` dichiarato FILE *f

Infatti:

int getc(FILE *stream);

Poi la cosa strana è che un paio di caratteri li prende, poi ad un certo punto dà segmentation fault e non si capisce perché...

[targzeta]

getc(f)

f e` dichiarato FILE *f

Dal man di getc trovato online ho che il prototipo è:

Codice: Seleziona tutto

int getc(FILE *stream);

Quindi non ci dovrebbero essere problemi. Ora non sono a casa e non posso testare il programma. Più tardi, se non avete ancora risolto, vedo di testarlo anche io.
Comunque una semplice accortezza, sarebbe giusto che il while sia

Codice: Seleziona tutto

while ( (c=getc(f)) != EOF )

Non è questo il problema, dato che EOF dovrebbe essere sempre a 0. Ma in generale questo codice non funzionerebbe se a EOF si decidesse di assegnare un altro valore. Insomma, è buona pratica usare queste accortezze.

Spina

P.S. A mio avviso l'errore non è lì.

[phobos3576]

Prova a sostituire:

Codice: Seleziona tutto

char *s;
s = malloc(2000);
char *p = s;

con:

Codice: Seleziona tutto

char s[2000];
char *p = s;

Ho anche dei dubbi (come Spina) su:

Codice: Seleziona tutto

while (c = getc(f))

La costante EOF spesso vale -1 per cui il while innesca un ciclo infinito con superamento dei limiti del vettore s.

Inoltre, come dicono K&R, non si dovrebbero mai scrivere programmi basati sul valore di EOF; si deve usare il nome simbolico EOF e basta.

[Mario Vanoni]

Mi meraviglia

char *v[MAX - 1];

in generis si mette
+1 per lo \0 finale.

[anycolouryoulike]

Prova a sostituire:

Codice: Seleziona tutto

char *s;
s = malloc(2000);
char *p = s;

con:

Codice: Seleziona tutto

char s[2000];
char *p = s;

Così funziona, ma perché?
Non è che funzioni perfettamente però, perché quando inizializza v[3], v[0] da "fadfa" diventa "ò¶´¿a".

Codice: Seleziona tutto

(gdb)
55                    v[++i -1] = p;
10: c = 100
9: v[3] = 0x66646166 <Address 0x66646166 out of bounds>
8: v[2] = 0xbfb4b6ed "gadg"
7: v[1] = 0xbfb4b6e6 "ahdahr"
6: v[0] = 0xbfb4b6e0 "fadfa"
5: newline = 0 '\0'
4: comment = 0 '\0'
3: word = 0 '\0'
2: i = 3
(gdb) p p
$1 = 0xbfb4b6f2 "\024"
(gdb) p *p
$2 = 20 '\024'
(gdb) s
56                    word = 1;
10: c = 100
9: v[3] = 0xbfb4b6f2 "\024"
8: v[2] = 0xbfb4b6ed "gadg"
7: v[1] = 0xbfb4b6e6 "ahdahr"
6: v[0] = 0xbfb4b6e0 "ò¶´¿a"
5: newline = 0 '\0'
4: comment = 0 '\0'
3: word = 0 '\0'
2: i = 4
(gdb)

Com'è possibile?

Ho anche dei dubbi (come Spina) su:

Codice: Seleziona tutto

while (c = getc(f))

La costante EOF spesso vale -1 per cui il while innesca un ciclo infinito con superamento dei limiti del vettore s.

L'ho corretto, ma l'errore non era dovuto a quello, perché il programma terminava prima della fine del file.

[targzeta]

Io ho fatto queste piccole modifiche:

Codice: Seleziona tutto

--- prova.orig.c        2009-01-19 20:32:22.000000000 +0100
+++ prova.new.c 2009-01-19 20:31:00.000000000 +0100
@@ -4,19 +4,21 @@
 
 #define MAX 4
 
-main(int argc, char *argv[])
+int main(int argc, char *argv[])
 {
   FILE *f;
   char *s;
-  char *v[MAX - 1];
+  char *v[MAX];
+  char *p;
+  int i, c;  
+  char newline, comment, word;
+
   f = fopen(argv[1], "r");
   s = malloc(2000);
-  char *p = s;
-  int i, c;
+  p=s;
   c = getc(f);
-  char newline, comment, word;
   newline = comment = word = i = 0;
-  while (c = getc(f))
+  while ( (c = getc(f)) != EOF )
     {
       if (c == '#')
        {
@@ -24,7 +26,7 @@
            comment = 1;
          continue;
        }
-      if (newline = (c == '\n'))
+      if ( (newline = (c == '\n')) )
        {
          if (comment)
            comment = 0;
@@ -54,7 +56,7 @@
        {
          if (! word)
            {
-             v[++i -1] = p;
+             v[i++] = p;
              word = 1;
            }
          *p++ = c;
@@ -62,4 +64,6 @@
     }
   for (i = 0; i < MAX; i++)
     puts(v[i]);
+
+  free(s)
+  return 0;
 }

E sembra funzionare. Diciamo che si vede che non sei proprio un guru del C .

Il problema dell'allocazione non c'entra, infatti non c'è differenza tra allocare memoria dinamicamente o staticamente a tempo di compilazione. Solo che se allochi memoria, poi devi liberarla (nota la free(s) alla fine)!!!
Inoltre, l'allocazione dinamica serve se non sai a priori quanto devi allocare, in questo caso invece sapevi già che l'array era grande 2*10^3B, quindi tanto valeva usare

Codice: Seleziona tutto

int s[2000]

come giustamente suggerito da phobos3576.

Non ho indagato bene nel codice e quindi può darsi che ci siano altri errori e che la mia sia solo fortuna. Comunque, a parte spostare le righe per renderle conformi con il C (ti consiglio di compilare sempre con -Wall -pedantic) quello che ho modificato è:

Codice: Seleziona tutto

-  while (c = getc(f))
+  while ( (c = getc(f)) != EOF )

-      if (newline = (c == '\n'))
+      if ( (newline = (c == '\n')) )

-             v[++i -1] = p;
+             v[i++] = p;

Ma solo perchè mi suonavano meglio, come ho detto non mi sono addentrato nell'algoritmo.

Spina

[robbybby]

Codice: Seleziona tutto

char *v[MAX]

Inoltre sarebbe bene inizializzare tutte le variabili durante la dichiarazione, per evitare che abbiano valori casuali.

Codice: Seleziona tutto

FILE *f = NULL;
char *s = NULL;
char *v[MAX];

#ifdef DEBUG   /o quello che e'
  memset(v,
         NULL,
         MAX * sizeof(char));
#endif

E poi dovresti controllare il valore ritornato dalla malloc()

Codice: Seleziona tutto

s = malloc(2000);

if(s == NULL)
  exit 1;

Probabilmente con

Codice: Seleziona tutto

char s[2000];

non crasha perche' aggiunge un po' di guardia sullo stack.

[Mario Vanoni]

Codice: Seleziona tutto

char *v[MAX]

v[MAX]
ma dove posizioni lo \0 finale?

v[MAX + 1];

[anycolouryoulike]

Io ho fatto queste piccole modifiche:

Codice: Seleziona tutto

--- prova.orig.c        2009-01-19 20:32:22.000000000 +0100
+++ prova.new.c 2009-01-19 20:31:00.000000000 +0100
@@ -4,19 +4,21 @@
 
 #define MAX 4
 
-main(int argc, char *argv[])
+int main(int argc, char *argv[])
 {
   FILE *f;
   char *s;
-  char *v[MAX - 1];
+  char *v[MAX];
+  char *p;
+  int i, c;  
+  char newline, comment, word;
+
   f = fopen(argv[1], "r");
   s = malloc(2000);
-  char *p = s;
-  int i, c;
+  p=s;
   c = getc(f);
-  char newline, comment, word;
   newline = comment = word = i = 0;
-  while (c = getc(f))
+  while ( (c = getc(f)) != EOF )
     {
       if (c == '#')
        {
@@ -24,7 +26,7 @@
            comment = 1;
          continue;
        }
-      if (newline = (c == '\n'))
+      if ( (newline = (c == '\n')) )
        {
          if (comment)
            comment = 0;
@@ -54,7 +56,7 @@
        {
          if (! word)
            {
-             v[++i -1] = p;
+             v[i++] = p;
              word = 1;
            }
          *p++ = c;
@@ -62,4 +64,6 @@
     }
   for (i = 0; i < MAX; i++)
     puts(v[i]);
+
+  free(s)
+  return 0;
 }

E sembra funzionare.

Scusa l'ignoranza ma come si usa questo codice?
Non sono pratico di patch...

Diciamo che si vede che non sei proprio un guru del C .

Ti ringrazio per l'informazione, anzi diciamo che lo sapevo già .
Questo è il secondo programma che scrivo (il primo è quest'altro: viewtopic.php?f=20&t=25469).

Codice: Seleziona tutto

char *v[MAX]

Inoltre sarebbe bene inizializzare tutte le variabili durante la dichiarazione, per evitare che abbiano valori casuali.

Questo mi era stato già detto.
Sarò testardo, ma non vedo perché dovrei inizializzarle subito anche se non ci accedo mai prima di inizializzarle...

Il problema dell'allocazione non c'entra, infatti non c'è differenza tra allocare memoria dinamicamente o staticamente a tempo di compilazione. Solo che se allochi memoria, poi devi liberarla (nota la free(s) alla fine)!!!
Inoltre, l'allocazione dinamica serve se non sai a priori quanto devi allocare, in questo caso invece sapevi già che l'array era grande 2*10^3B, quindi tanto valeva usare

Codice: Seleziona tutto

int s[2000]

come giustamente suggerito da phobos3576.

Ma infatti non lo so quanto dovrò memorizzare.
Perché 2*10^3B? Cos'è B?

[targzeta]

...
v[MAX]
ma dove posizioni lo \0 finale?

v[MAX + 1];

Mario, la forma a cui ti riferisci serve se uno la vuole usare, non è mandatory (nota che v è un array di puntatore, quindi il '\0' ha poco senso). In questo caso lui dice di vole usare tutto l'array v[MAX] (con l'accortezza di scrivere sino a v[MAX-1], ovvero l'array contiene MAX elementi da 0 a MAX-1), ecco perchè, presumibilmente, non bana a salvarsi da qualche parte il numero di elementi inseriti nell'array (infatti la variabile 'i' viene azzerata all'uscita del while rendendo impossibile sapere se l'array v è stato completamente riempito o meno).

Il tuo metodo implicherebbe di impostare a NULL (o a -1 o a '\0', o a qualsiasi altra cosa) l'ultimo elemento significativo dell'array e poi ciclare sino a che non si trova NULL (o -1, o '\0' o qualsiasi altra cosa). Non è che si debba per forza fare così!

Spina

[targzeta]

...Scusa l'ignoranza ma come si usa questo codice?

Ci sono righe il cui primo carattere è diverso dai caratteri '+' o '-'. Queste righe non le considerare. Le righe che iniziano con il '-' sono le righe del tuo file originale che andrebbero eliminate e sostituite con le righe che iniziano con il '+'.

Per fare questo in automatico potresti copiare e incollare il codice in un file, ad esempio casa.patch, ed eseguire:

Codice: Seleziona tutto

patch file_originale.c casa.patch

Ma molto probabilmente non funzionerebbe per via della formattazione strampalata che esce qui sul forum. Quindi semplicemente dai un occhiata alle modifiche che ho apportato.

Comunque ti allego il file che ho scritto io e che sembra funzionare.

Spina

[Mario Vanoni]

...
v[MAX]
ma dove posizioni lo \0 finale?

v[MAX + 1];

Mario, la forma a cui ti riferisci serve se uno la vuole usare, non è mandatory (nota che v è un array di puntatore, quindi il '\0' ha poco senso). In questo caso lui dice di vole usare tutto l'array v[MAX] (con l'accortezza di scrivere sino a v[MAX-1], ovvero l'array contiene MAX elementi da 0 a MAX-1), ecco perchè, presumibilmente, non bana a salvarsi da qualche parte il numero di elementi inseriti nell'array (infatti la variabile 'i' viene azzerata all'uscita del while rendendo impossibile sapere se l'array v è stato completamente riempito o meno).

Il tuo metodo implicherebbe di impostare a NULL (o a -1 o a '\0', o a qualsiasi altra cosa) l'ultimo elemento significativo dell'array e poi ciclare sino a che non si trova NULL (o -1, o '\0' o qualsiasi altra cosa). Non è che si debba per forza fare così!

Spina

Precisazione, siamo generazioni diverse!

Come insegnavano Pike & Co. e` prudente
mettere una sentinella alla fine di un array.

Usavo sempre "~~~", ultimo segno ASCII,
controllando con un printf l'ultimo elemento,
in fase di test sei sicuro che tutto e` in ordine.

Sei in produzione poi qualcuno segnala un "~~~",
sai che eri troppo sicuro, ma hai un hint dove cercare.

[anycolouryoulike]

Si la tua versione funziona.
In sostanza hai aggiunto un elemento apparentemente inutile, che di fatto non viene mai utilizzato (e neanche stampato), al vettore v.
Dico apparentemente perché in realtà se si toglie quell'elemento dall'array si ripresenta esattamente lo stesso problema che ho descritto io.
Ora la mia domanda è: perché getc genera un segmentation fault in mancanza di un elemento inutilizzato in un array di puntatori che non c'entra niente con la variabile in cui memorizza il carattere?
È abbastanza strano no?

...
v[MAX]
ma dove posizioni lo \0 finale?

v[MAX + 1];

Mario, la forma a cui ti riferisci serve se uno la vuole usare, non è mandatory (nota che v è un array di puntatore, quindi il '\0' ha poco senso). In questo caso lui dice di vole usare tutto l'array v[MAX] (con l'accortezza di scrivere sino a v[MAX-1], ovvero l'array contiene MAX elementi da 0 a MAX-1), ecco perchè, presumibilmente, non bana a salvarsi da qualche parte il numero di elementi inseriti nell'array (infatti la variabile 'i' viene azzerata all'uscita del while rendendo impossibile sapere se l'array v è stato completamente riempito o meno).

Il tuo metodo implicherebbe di impostare a NULL (o a -1 o a '\0', o a qualsiasi altra cosa) l'ultimo elemento significativo dell'array e poi ciclare sino a che non si trova NULL (o -1, o '\0' o qualsiasi altra cosa). Non è che si debba per forza fare così!

Spina

Invece sembra proprio che quell'elemento sovrabbondante sia necessario!
E anche tu l'hai inserito nella tua versione, anche se non l'hai mai inizializzato.
Perché?