Thread: program compiles, runs, but produces no output

Well, here it is. I am having problems only within the for loop at the bottom(in red). I think there could be an error in logic.

Code:

#define _CRT_SECURE_NO_WARNINGS // To turn off VC++ 2008 deprecation warnings

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

#define NUMBEROFFILES 1500		// maximum number of files in the directory listing
#define LINELENGTH 150			// length of a line in the directory listing
#define FILENAMELENGTH 40		// length of an .html file
#define BEGINNINGOFHTMLLIST 5	// first html file in directory list
#define FILELINELENGTH 1000		// lengh of a line in an html file

int main( void )
{ 
    int lastfilenumber1, lastfilenumber2;
    FILE *p_openfile;
	char *p_comma;				// used to test for occurence of a comma in a line of the directory listing
	char *p_line;				// stores successive lines from a file from the directory listing
    int  counter1, counter2;
	char *p_searchstring;		// used for strstr()
	char *p_searchtext = "every";		// string to search for in a file

	struct data {
        int month, day, year, hour, minute;
        char string1[2+1];
		int number1, number2;
        char string2[FILENAMELENGTH];
        char stringstorage[LINELENGTH];		// Used to store strings from file
		char string3[FILENAMELENGTH];		// filenames that contain the search string
    } LineData[NUMBEROFFILES];

    // 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( (p_openfile = fopen("directorylisting.txt", "rt")) == NULL)
    {
        perror("directorylisting.txt");
        exit( EXIT_FAILURE );
    }

    // Copy the lines in the file to the array 

    counter1 = 0;

    while((fgets(LineData[counter1].stringstorage, LINELENGTH, p_openfile) != NULL) && (counter1 < NUMBEROFFILES))
    {
		lastfilenumber1 = counter1;
        counter1++;	
    }

    fclose(p_openfile);

	// Process the string, and store data items in the structure members

	for( counter1 = BEGINNINGOFHTMLLIST; counter1 < lastfilenumber1; counter1++)
	{
		if( (p_comma = strchr( LineData[counter1].stringstorage, ',')) != NULL )
		{
			sscanf(LineData[counter1].stringstorage, "%d%/%d/%d %d:%d %s %d,%d %s", &LineData[counter1].month,
				&LineData[counter1].day,&LineData[counter1].year, &LineData[counter1].hour,
				&LineData[counter1].minute, LineData[counter1].string1, &LineData[counter1].number1,
				&LineData[counter1].number2,&LineData[counter1].string2);
		}

		if( (p_comma = strchr( LineData[counter1].stringstorage, ',')) == NULL )
		{
			sscanf(LineData[counter1].stringstorage, "%d%/%d/%d %d:%d %s %d %s", &LineData[counter1].month,
				&LineData[counter1].day, &LineData[counter1].year, &LineData[counter1].hour,
				&LineData[counter1].minute, LineData[counter1].string1, &LineData[counter1].number1,
				&LineData[counter1].string2);
		}
	}

	// open up a file, search for p_searchtext, if it exists, store filename(LineData[counter1].string2) in
	// LineData[counter2].string3 then print LineData[counter2].string3. open up next file etc.

	counter2 = 0;

	for( counter1 = BEGINNINGOFHTMLLIST; counter1 < (lastfilenumber1 - 1); counter1++) 
	{	
		if((p_openfile = fopen(LineData[counter1].string2, "rt")) == NULL)
		{
			printf("\nError opening file %s", LineData[counter1].string2);
			exit(EXIT_FAILURE);
		}

		p_line = malloc(FILELINELENGTH * sizeof(char));

		while((fgets(p_line, FILELINELENGTH, p_openfile) != NULL) && 
			(p_searchstring = strstr(p_line, p_searchtext) != NULL)) 	
		{
			
			strcpy(LineData[counter2].string3, LineData[counter1].string2); 
			printf("\n%s", LineData[counter2].string3);
			lastfilenumber2 = counter1;
			counter2++;
			free(p_line);
			p_line = malloc(FILELINELENGTH * sizeof(char));
		}

		fclose(p_openfile);
	}

    return 0;
}

Here is what I want to achieve with the for loop:

1. open an .html file for reading.
2. allocate memory to a pointer to a string(p_line)
3. use fgets() to read the first string(line) of the file into p_line
4. search p_line for the occurence of 'every'
5. if p_line(and therefore the file) contains the word 'every', copy that filename to LineData[counter2].string3, then printf LineData[counter2].string3
6. free memory to p_line
7. allocate memory to p_line again
8. go to step 3 to read the next line in the file
9. go to 4.
10. go to 5...and so on.
11. keep reading files until the EOF
12. go to 1 again to open up the next file and repeat until all files have been opened and searched.

What I am really trying to achieve is to print the names of all the files that contain the word 'every'.

However, nothing gets printed at all.

Where did I go wrong?

Thanks in advance.

Code:

		p_line = malloc(FILELINELENGTH * sizeof(char));

		while((fgets(p_line, FILELINELENGTH, p_openfile) != NULL) && 
			(p_searchstring = strstr(p_line, p_searchtext) != NULL)) 	
		{
			
			strcpy(LineData[counter2].string3, LineData[counter1].string2); 
			printf("\n%s", LineData[counter2].string3);
			lastfilenumber2 = counter1;
			counter2++;
			free(p_line);
			p_line = malloc(FILELINELENGTH * sizeof(char));
		}

RED: You probably should not allocate a FILELINELENGTH variable every loop - and if you still feel you must do it, please free the last one after the loop. But it would be perfectly fine to allocate before and free after the loop (or just have a stack-based array of 1000 chars).

BLUE: So you want the loop to end if you don't find your search-text?

Code:

		if( (p_comma = strchr( LineData[counter1].stringstorage, ',')) != NULL )
		{
			sscanf(LineData[counter1].stringstorage, "%d%/%d/%d %d:%d %s %d,%d %s", &LineData[counter1].month,
				&LineData[counter1].day,&LineData[counter1].year, &LineData[counter1].hour,
				&LineData[counter1].minute, LineData[counter1].string1, &LineData[counter1].number1,
				&LineData[counter1].number2,&LineData[counter1].string2);
		}

		if( (p_comma = strchr( LineData[counter1].stringstorage, ',')) == NULL )
		{
			sscanf(LineData[counter1].stringstorage, "%d%/%d/%d %d:%d %s %d %s", &LineData[counter1].month,
				&LineData[counter1].day, &LineData[counter1].year, &LineData[counter1].hour,
				&LineData[counter1].minute, LineData[counter1].string1, &LineData[counter1].number1,
				&LineData[counter1].string2);
		}

The red and the blue line are complete opposites. Do you expect the data to change during the first ssacnf() for some reason, or are you just wanting to spend some extra time using strchr() to not find what you are looking for in one case, rather than using a simple

Code:

if (strchr(...) != NULL) ... 
else 
    // strchr(...) is NULL

It also looks (to me) like your two cases have only the format string different, and that the call to sscanf() is identical in the two if-statements. If that is the case, then I would probably do something like this:

Code:

const char *fmt = "%d%/%d/%d %d:%d %s %d,%d %s";  // strchr() != NULL variant. 
if (strchr(...) == NULL)
   fmt = "%d%/%d/%d %d:%d %s %d %s";
sscanf(..., fmt, ...);

That way, the code is much simpler to follow, and the long argument list of ALMOST the same call only appears once in the code. Note that this assumes that the arguments to scanf are actually EXACTLY IDENTICAL, rather than simply very similar - I may have it wrong there.

--
Mats

Originally Posted by matsp

RED: You probably should not allocate a FILELINELENGTH variable every loop - and if you still feel you must do it, please free the last one after the loop. But it would be perfectly fine to allocate before and free after the loop (or just have a stack-based array of 1000 chars).

I'll need to think more about this one.

BLUE: So you want the loop to end if you don't find your search-text?

If the search for the world 'every' in a string is successful, I would like to break out of the loop, close the file, and to open a new file for searching. Otherwise, use fgets to read a new line in the current file into p_line and search for 'every' again.

The red and the blue line are complete opposites. Do you expect the data to change during the first ssacnf() for some reason, or are you just wanting to spend some extra time using strchr() to not find what you are looking for in one case, rather than using a simple

It also looks (to me) like your two cases have only the format string different, and that the call to sscanf() is identical in the two if-statements. If that is the case, then I would probably do something like this:

The two if's are not a problem. They are used to scan the below lines(in the input file). In the 4th column, the number is either a digit(969), or a digit+comma+digit(10,261). The two ifs test either condition, and appropriately use the correct number of identifiers to sscanf the data. It works perfectly, and is not a part of the problem.

12/16/2008 10:21 AM 969 bobthebuilder.html
08/14/2008 06:13 AM 10,261 purplebarney.html
08/14/2008 06:08 AM 12,083 mrpotatohead.html
08/09/2008 05:10 PM 4,076 spongebob.html
08/09/2008 05:41 PM 3,531 thewiggles.html
12/06/2008 03:04 PM 1,016 lindsaylohan.html

My post #6(and later) in this thread explains it:

http://cboard.cprogramming.com/showt...769#post828769

The more I think about it, this seems to be the problem, because everything up to it has worked fine many times.

Code:

p_line = malloc(FILELINELENGTH * sizeof(char));

	while((fgets(p_line, FILELINELENGTH, p_openfile) != NULL) && 
			(p_searchstring = strstr(p_line, p_searchtext) != NULL)) 	
		{
			
			strcpy(LineData[counter2].string3, LineData[counter1].string2); 
			printf("\n%s", LineData[counter2].string3);
			lastfilenumber2 = counter1;
			counter2++;
			free(p_line);
			p_line = malloc(FILELINELENGTH * sizeof(char));
		}

I forgot to add the compiler warnings:

(94) : warning C4047: '=' : 'char *' differs in levels of indirection from 'int'
(94) : warning C4706: assignment within conditional expression

which both point to this line(while condition):

Code:

while((fgets(p_line, FILELINELENGTH, p_openfile) != NULL) && (p_searchstring = strstr(p_line, p_searchtext) != NULL)) 	
	{
			
	strcpy(LineData[counter2].string3, LineData[counter1].string2); 
	printf("\n%s", LineData[counter2].string3);  <=== why doesn't anything print at all?
	lastfilenumber2 = counter1;
	counter2++;
	free(p_line);
	p_line = malloc(FILELINELENGTH * sizeof(char));
		}

My question really is: why doesn't the printf statement in the above line print? Even if there is gargabe in
LineData[counter2].string3, something should still print, right?

I didn't mean to say that the two if's was a direct problem - it is a problem in the sense that there is needless duplication of code (at least the strchr() call).

As to why there is no output, perhaps I suspect it is a problem with your strstr() statement. Split that out into a

Code:

if (strstr(...)) 
{
    // Maybe add a printf here.
    break;
}

--
Mats

Originally Posted by matsp

I didn't mean to say that the two if's was a direct problem - it is a problem in the sense that there is needless duplication of code (at least the strchr() call).

Ah, I got you now. Makes perfect sense.

As to why there is no output, perhaps I suspect it is a problem with your strstr() statement. Split that out into a [code]
if (strstr(...))
{
// Maybe add a printf here.
break;
}

Sweet Jesus! It works perfect!

Code:

while((fgets(p_line, FILELINELENGTH, p_openfile)) != NULL) 	
		{
			if( (p_searchstring = strstr(p_line, p_searchtext) != NULL))
			{
strcpy(LineData[counter2].string3, LineData[counter1].string2); 
printf("\n%s", LineData[counter2].string3);
break;
			}

			lastfilenumber2 = counter1;
			counter2++;
			free(p_line);
			p_line = malloc(FILELINELENGTH * sizeof(char));
		}

Problems solved! Thanks mats!

--
Mats[/QUOTE]

So, I was on the right track with "do you want your while to stop on this condition" - unless the expression is REALLY simple, I actually prefer to add a if ... break bit of code to the loop, rather than having a complex condition - it makes the code easier to read, and this in turn makes it easier to understand why it doesn't work when something goes wrong.

--
Mats

Well, I tested on 5 .html files, of which 2 contained the word 'every', and it successfully printed out the 2 filenames with that word.

Then I got really sadistic and tested it on 938 files, of which there are at least 400 files containing the word 'every'.

It managed to print out 25 names of files with that word, then crashes.

Do you think the compiler warnings had anything to do with it?

(94) : warning C4047: '=' : 'char *' differs in levels of indirection from 'int'
(94) : warning C4706: assignment within conditional expression

They both apply to this line in the while loop:

Code:

while((fgets(p_line, FILELINELENGTH, p_openfile)) != NULL) 	
		{
			if( (p_searchstring = strstr(p_line, p_searchtext) != NULL))
			{
				strcpy(LineData[counter2].string3, LineData[counter1].string2); 
				printf("\n%s", LineData[counter2].string3);
				break;
			}

Thanks.

EDIT: Hmm..I tested it on 111 files, and it still crashes after printing out 25 files.

EDIT: Ok, now I tested it on 74 files, and it still crashes after printing out 25 files.

EDIT: Ok, now I tested it on 37 files, and it still crashes after printing out 25 files.

EDIT: Ok, now I tested it on 10 files, and it successfully prints out 7 filenames that contain the word 'every'.

EDIT: Ok, now I tested it on 20 files, and it successfully prints out 17 filenames that contain the word 'every'.

EDIT: Now I am going to sleep. Goodnight. Zzzzzz.....

A lot of your code appears to be related to the parsing of the directory listing - have you tried using "dir /b" which produces a "bare"format, with only the filename - or do you actually NEED the other data that you get from the directory listing?

And you still have a memory leak in your read-loop, and more importantly, you do not check for NULL from malloc - so it would crash if that malloc fails.

--
Mats

Originally Posted by matsp

have you tried using "dir /b" which produces a "bare"format, with only the filename.

AAAarrgghhh!! If I had know about "dir /b", I wouldn't have written so much code to process and store at least 3600 data items for which I have zero use. All I had wanted was the filename.

And you still have a memory leak in your read-loop, and more importantly, you do not check for NULL from malloc - so it would crash if that malloc fails.

I don't know how to stop a memory leak, but I will apply a check for the malloc.

I will work on it, and will report back if I have any problems.

Thanks heaps for your help.

--
Mats[/QUOTE]

To stop the memory leak, just do what I originally suggested: Allocate before the loop, and free after the loop - no allocation inside the loop, so no problem to know if you have freed it in the loop or not.

--
Mats

Originally Posted by matsp

To stop the memory leak, just do what I originally suggested: Allocate before the loop, and free after the loop - no allocation inside the loop, so no problem to know if you have freed it in the loop or not.

So when allocating memory to a pointer with malloc, the allocation needs to be done only once? The program keeps using that allocated memory again and again until it is freed? I always thought I had to keep reallocating(and freeing) the memory every time I read a new string into the same pointer. Which was why I initially allocated and free memory within the while loop.

EDIT: Are you talking about allocating outside of ALL loops, or just outside the while loop? I got the impression you were talking about the while loop. I've since done both, and the results are the same, either way.

Ok, here's the code so far, but the program still crashes after printing out 25 filenames that contains the search string.

Code:

#define _CRT_SECURE_NO_WARNINGS // To turn off VC++ 2008 deprecation warnings

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

#define NUMBEROFFILES 1500		// maximum number of files in the directory listing
#define LINELENGTH 150			// length of a line in the directory listing
#define FILENAMELENGTH 40		// length of an .html file
#define BEGINNINGOFHTMLLIST 5	// first html file in directory list
#define FILELINELENGTH 1000		// lengh of a line in an html file

int main( void )
{ 
    int lastfilenumber1, lastfilenumber2;
    FILE *p_openfile;
	char *p_line;	// stores successive lines from a file from the directory listing
	char *p_newline;
    int  counter1, counter2 = 0;
	char *p_searchstring;		// used for strstr()
	char *p_searchtext = "every";		// string to search for in a file

	struct data {
		int number1, number2;
        char string2[FILENAMELENGTH];		// stores filenames from directory listing
        char storefilename[LINELENGTH];		// Used to store strings from file
		char string3[FILENAMELENGTH];		// filenames that contain the search string
    } LineData[NUMBEROFFILES];

    // Get a list of files in the directory "testwebsite", and redirect the
    // output to a file called "directorylisting.txt". */

	system("dir /b c:\\testwebsite\\*.html > directorylisting.txt");

    // Test to see that the file "directorylisting.txt" can be opened for reading.

    if( (p_openfile = fopen("directorylisting.txt", "rt")) == NULL)
    {
        perror("directorylisting.txt");
        exit( EXIT_FAILURE );
    } 

    counter1 = 0;

	// Copy the lines(filenames) in the file to the array

    while((fgets(LineData[counter1].storefilename, LINELENGTH, p_openfile) != NULL) && (counter1 < NUMBEROFFILES))
    {

		if((p_newline = strchr(LineData[counter1].storefilename, '\n')) != NULL)
			*p_newline = '\0';

        counter1++;	
		lastfilenumber1 = counter1;
    }

    fclose(p_openfile);

	// open up a file, search for p_searchtext, if it exists, store filename(LineData[counter1].storefilename)
	// in LineData[counter2].string3 then print LineData[counter2].string3. open up next file etc.

	for( counter1 = 0; counter1 < lastfilenumber1; counter1++) 
	{	
		if((p_openfile = fopen(LineData[counter1].storefilename, "r")) == NULL)
		{
			perror("Error opening file");
			exit(EXIT_FAILURE);
		}

		p_line = malloc(FILELINELENGTH * sizeof(char));

		if( p_line == NULL )
		{
			perror("Error allocating memory");
			exit (EXIT_FAILURE);
		}

		while((fgets(p_line, FILELINELENGTH, p_openfile)) != NULL) 	
		{
			if( (p_searchstring = strstr(p_line, p_searchtext) != NULL))
			{
				strcpy(LineData[counter2].string3, LineData[counter1].storefilename); 
				printf("\n%s", LineData[counter2].string3);
				break;
			}

			lastfilenumber2 = counter1;
			counter2++;
		}

		free(p_line);

		fclose(p_openfile);
	}

    return 0;
}