accessing a malloc'd array problem.
Author |
Message |
Evan Clark #1 / 7
|
accessing a malloc'd array problem.
Hello all. I have a problem when trying to access or free an array which I have malloc'd. Here is a snippet that should hopefully highlight the problem. The snippet loads a text file into memory, and indexes it via an array of pointers to the start of each line. It then displays the file contents line by line, before printing up some stats and exiting. Here is the code: #include <stdio.h> #include <stdlib.h> FILE *in; long int length; int counter; char *data = NULL; char **item = NULL; int numlines = 0; int main () { /* Open the vendors file for reading */ if( (in = fopen ("VENDORS", "r")) == NULL) { fprintf (stderr, "Cannot open file VENDORS.\n"); exit (-1); } /* Get the length of the file in bytes */ /* NOT PORTABLE */ fseek (in, 0, SEEK_END); length = ftell (in); rewind (in); /* Allocate memory for the file contents */ if ( (data = malloc (length)) == NULL) { fprintf (stderr, "Cannot allocate %ld bytes (data).\n", length); exit (EXIT_FAILURE); } /* Read the file into memory */ if ( (fread (data, sizeof (char), length, in)) != (unsigned long int)length) { fprintf (stderr, "Error reading from file.\n"); exit (EXIT_FAILURE); } fclose (in); /* Count the number of lines */ for (counter = 0; counter < length; counter++) if (*(data + counter) == '\n') numlines++; /* Allocate an array of pointers to a char */ /* This is to hold the address of the start of each line */ if ( (item = malloc(numlines * sizeof(*item))) == NULL) { fprintf (stderr, "Could not allocate %d bytes (item).\n", numlines * sizeof(char *)); exit (EXIT_FAILURE); } /* The first line starts at the first character */ item[0] = data; numlines = 0; /* Set up the array for each line and additionally change each */ /* '\n' to a '\0' */ for (counter = 0; counter < length; counter++) if (data[counter] == '\n') { numlines++; data[counter] = '\0'; item[numlines] = &data[counter+1]; } /* Display the file line by line */ for (counter = 0; counter < numlines; counter ++) printf ("%s\n", item[counter]); /* Free allocated memory */ free (item); free (data); /* Just some stats for debugging */ printf ("%ld, %d\n", length, numlines); return 0; Quote: }
It segfaults when accessing the array contents via the printf for displaying the file's contents in memory. If I comment that out, it also segfaults at the free (item); Any ideas?
|
Sat, 02 Apr 2005 16:38:17 GMT |
|
|
Ga?l Le Mign #2 / 7
|
accessing a malloc'd array problem.
> /* Read the file into memory */ > if ( (fread (data, sizeof (char), length, in)) != (unsigned > long int)length) > { > fprintf (stderr, "Error reading from file.\n"); > exit (EXIT_FAILURE); > } > fclose (in); pay attention, the one that answer to read() (the kernel or the server depending of the system) isn't bound to give you all the data at once, for a reason or another you may need to loop while fread() is not 0... It's not needed usually, but it's suggested. > /* Allocate an array of pointers to a char */ > /* This is to hold the address of the start of each line */ > if ( (item = malloc(numlines * sizeof(*item))) == NULL) so, you've malloc'ed() an array of size numlines... since your numlines start from 0, you have lines from 0 to numlines, including 0 and numlines. So you have in fact numines + 1 lines in your program, and you array is too small. It's easy to find those kind of bugs by using Electric Fence or Valgrind and a good de{*filter*} (like gdb) --
GSM : 06.71.47.18.22 (in France) ICQ UIN : 7299959 Fingerprint : 1F2C 9804 7505 79DF 95E6 7323 B66B F67B 7103 C5DA Member of HurdFr: http://www.*-*-*.com/ - The GNU Hurd: http://www.*-*-*.com/
|
Sat, 02 Apr 2005 17:04:44 GMT |
|
|
Evan Clark #3 / 7
|
accessing a malloc'd array problem.
Quote:
> > /* Read the file into memory */ > > if ( (fread (data, sizeof (char), length, in)) != (unsigned > > long int)length) > > { > > fprintf (stderr, "Error reading from file.\n"); > > exit (EXIT_FAILURE); > > } > > fclose (in); > pay attention, the one that answer to read() (the kernel or the server > depending of the system) isn't bound to give you all the data at once, > for a reason or another you may need to loop while fread() is not 0... > It's not needed usually, but it's suggested. > > /* Allocate an array of pointers to a char */ > > /* This is to hold the address of the start of each line */ > > if ( (item = malloc(numlines * sizeof(*item))) == NULL) > so, you've malloc'ed() an array of size numlines... since your numlines > start from 0, you have lines from 0 to numlines, including 0 and numlines. > So you have in fact numines + 1 lines in your program, and you array > is too small. > It's easy to find those kind of bugs by using Electric Fence or Valgrind > and a good de{*filter*} (like gdb)
Thank you. It worked a treat. Just out of interest: how would I track this down in gdb? It appeared to be working fine (even allocating values to the out-of-range array element), until I tried to free it. So I assume it is a stack corruption issue? I was using gdb to see that things were pointing to where they should be, and they were.
|
Sat, 02 Apr 2005 17:14:10 GMT |
|
|
Ga?l Le Mign #4 / 7
|
accessing a malloc'd array problem.
> Thank you. It worked a treat. Just out of interest: how would I > track this down in gdb? Use electric fence or valgrind (both are free software, apt-get'able in the Debian Sid, included in most other distributions, and you can find them with google easily else) > It appeared to be working fine (even allocating values to the > out-of-range array element), until I tried to free it. So I assume it > is a stack corruption issue? Not exactly a stack corruption problem, but a malloc internal structure corruption. malloc() uses internal structure to keep track of free and allocated blocks, size of blocks, ... When you write outside a malloc'ed array, you can easily corrupt those data, and then the next malloc() or free() segfaults. > I was using gdb to see that things were pointing to where they should > be, and they were. Use eletric fence (install it, and link your programe with efence with gcc -lefence), it will make your program segfaults as soon as you do an overflow (well, not always, but must of the time, read the manual page first ;) ) --
GSM : 06.71.47.18.22 (in France) ICQ UIN : 7299959 Fingerprint : 1F2C 9804 7505 79DF 95E6 7323 B66B F67B 7103 C5DA Member of HurdFr: http://hurdfr.org - The GNU Hurd: http://hurd.gnu.org
|
Sat, 02 Apr 2005 17:27:02 GMT |
|
|
Evan Clark #5 / 7
|
accessing a malloc'd array problem.
Quote:
> Use electric fence or valgrind (both are free software, apt-get'able in the > Debian Sid, included in most other distributions, and you can find them > with google easily else)
Thanks for the pointer and background information - unfortunatily I use cygwin for windows for gcc and the like, and Electric Fence does not like it. Oh well.
|
Sat, 02 Apr 2005 21:10:10 GMT |
|
|
Barry Schwar #6 / 7
|
accessing a malloc'd array problem.
Quote:
> > /* Read the file into memory */ > > if ( (fread (data, sizeof (char), length, in)) != (unsigned > > long int)length) > > { > > fprintf (stderr, "Error reading from file.\n"); > > exit (EXIT_FAILURE); > > } > > fclose (in); >pay attention, the one that answer to read() (the kernel or the server >depending of the system) isn't bound to give you all the data at once, >for a reason or another you may need to loop while fread() is not 0... >It's not needed usually, but it's suggested.
According to the standard, fread can return a value less than the requested number of elements (length in this case) only if a read error or end of file is encountered. If the kernel or server does what you describe, it is up to the fread implementation to do the "looping" so you never have to. Quote: > > /* Allocate an array of pointers to a char */ > > /* This is to hold the address of the start of each line */ > > if ( (item = malloc(numlines * sizeof(*item))) == NULL) >so, you've malloc'ed() an array of size numlines... since your numlines >start from 0, you have lines from 0 to numlines, including 0 and numlines. >So you have in fact numines + 1 lines in your program, and you array >is too small.
The OP's code counted every '\n'. Why do you think there is another line? <<Remove the del for email>>
|
Sun, 03 Apr 2005 09:06:47 GMT |
|
|
Barry Schwar #7 / 7
|
accessing a malloc'd array problem.
On Tue, 15 Oct 2002 18:38:17 +1000, Evan Clarke Quote:
>Hello all. >I have a problem when trying to access or free an array which I have >malloc'd. Here is a snippet that should hopefully highlight the problem. >The snippet loads a text file into memory, and indexes it via an array >of pointers to the start of each line. It then displays the file >contents line by line, before printing up some stats and exiting. >Here is the code: >#include <stdio.h> >#include <stdlib.h> >FILE *in; >long int length; >int counter; >char *data = NULL; >char **item = NULL; >int numlines = 0; >int main () >{ > /* Open the vendors file for reading */ > if( (in = fopen ("VENDORS", "r")) == NULL) > { > fprintf (stderr, "Cannot open file VENDORS.\n"); > exit (-1); > } > /* Get the length of the file in bytes */ > /* NOT PORTABLE */ > fseek (in, 0, SEEK_END); > length = ftell (in); > rewind (in); > /* Allocate memory for the file contents */ > if ( (data = malloc (length)) == NULL) > { > fprintf (stderr, "Cannot allocate %ld bytes (data).\n", length); > exit (EXIT_FAILURE); > } > /* Read the file into memory */ > if ( (fread (data, sizeof (char), length, in)) != (unsigned long >int)length) > { > fprintf (stderr, "Error reading from file.\n"); > exit (EXIT_FAILURE); > } > fclose (in); > /* Count the number of lines */ > for (counter = 0; counter < length; counter++)
counter is an int; length is a long. If length contains a value larger than INT_MAX, you will invoke undefined behavior. Quote: > if (*(data + counter) == '\n') > numlines++; > /* Allocate an array of pointers to a char */ > /* This is to hold the address of the start of each line */ > if ( (item = malloc(numlines * sizeof(*item))) == NULL) > { > fprintf (stderr, "Could not allocate %d bytes (item).\n", numlines * >sizeof(char *)); > exit (EXIT_FAILURE); > } > /* The first line starts at the first character */ > item[0] = data; > numlines = 0; > /* Set up the array for each line and additionally change each */ > /* '\n' to a '\0' */ > for (counter = 0; counter < length; counter++) > if (data[counter] == '\n') > { > numlines++; > data[counter] = '\0'; > item[numlines] = &data[counter+1]; > }
If the last char in the file is a '\n', this will attempt to initialize a pointer beyond the end of item. If it is not a '\n', then you will never insert a '\0' to terminate the string. In either case, it is undefined behavior (here or in the printf call in the next loop). Quote: > /* Display the file line by line */ > for (counter = 0; counter < numlines; counter ++) > printf ("%s\n", item[counter]); > /* Free allocated memory */ > free (item); > free (data); > /* Just some stats for debugging */ > printf ("%ld, %d\n", length, numlines); > return 0; >} >It segfaults when accessing the array contents via the printf for >displaying the file's contents in memory. >If I comment that out, it also segfaults at the free (item); >Any ideas?
<<Remove the del for email>>
|
Sun, 03 Apr 2005 12:48:49 GMT |
|
|
|