Originally Posted by
Salem
Code:
while( (files[counter] = (char *)malloc( FILENAMESIZE * sizeof(char))) &&
(fgets(files[counter], BUFSIZ, openfile) != NULL))
1. There's no need to cast the result of malloc in a C program - see the FAQ.
2. Use the same size for allocating and reading. If BUFSIZ > FILENAMESIZE, then your program is on very thin ice.
3. At the end of the file, you allocate a dummy line which never gets used.
4. You don't free any of the memory.
5. The while loop should also limit counter < NUMBEROFFILES
Thanks for that. .
For #4, if I have an array of pointers ie. char *files[NUMBEROFFILES]; can I just free it with free(files)? I know how to free a memory allocated to pointer to type char, but I've never had to do it for an array of pointers to char. Does free() work exactly the same way?
I've been giving some thought to the while() loop, and I've got some questions.
Code:
while( (files[counter] = (char *)malloc( FILENAMESIZE * sizeof(char))) && (fgets(files[counter], BUFSIZ, openfile) != NULL))
{
counter++;
lastfilenumber = counter;
}
1. The first part:
Code:
(files[counter] = (char *)malloc( FILENAMESIZE * sizeof(char)))
is not really a condition? It's an assignment? Should there always be a condition(not assignment) in a while loop(or any other loop)?So while the above syntax works, is it technically incorrect?
2. I want to test that malloc successfully allocated the memory in the while() loop, but why does this:
Code:
while( (files[counter] = malloc( FILENAMESIZE * sizeof(char)) != NULL) && (fgets(files[counter], BUFSIZ, openfile) != NULL))
give me a compiler error message:
31 c:\examples\programs\test.c
warning: assignment makes pointer from integer without a cast
Also, in it's current form, is:
Code:
if( files[counter] == NULL )
{
perror("\nMemory allocation error");
exit( EXIT_FAILURE );
}
a reliable method of testing if malloc was successful, given that the test was not done in the while() loop?
Anyway, here's the cleaned up code, can you see a problem with anything else? Thanks.
Code:
#include <stdlib.h>
#include <stdio.h>
#define NUMBEROFFILES 1500
#define FILENAMESIZE 500
int main( void )
{
/* Array of pointers to store filenames */
char *files[NUMBEROFFILES];
int counter, lastfilenumber;
FILE *openfile;
char *string;
/* Get a list of files in the directory "testwebsite", and redirect the
output to a file called "directorylisting.txt". */
system("dir c:\\testwebsite\\*.html > directorylisting.txt");
/* Test to see that the file "directorylisting.txt" can be opened for reading. */
if( (openfile = fopen("directorylisting.txt", "rt")) == NULL)
{
perror("\ndirectorylisting.txt");
exit( EXIT_FAILURE );
}
counter = 1;
while( (files[counter] = malloc( FILENAMESIZE * sizeof(char))) && (fgets(files[counter], FILENAMESIZE, openfile) != NULL)
&& (counter < NUMBEROFFILES))
{
counter++;
lastfilenumber = counter;
}
if( files[counter] == NULL )
{
perror("\nMemory allocation error");
exit( EXIT_FAILURE );
}
fclose(openfile);
/* Print the strings in the array of pointers */
for( counter = 1; counter < lastfilenumber; counter++)
printf("Filename %d: %s", counter, files[counter]);
free(files);
return 0;
}