Help getting runtime double free or corruption error

I thought I had this done, but if I try to run with multiple files (mygrep print *.c) I get a double free or corruption runtime error. Any suggestions.

Code:

/*
 *CS306 Lab 1 lab1.c
 *Author: Chris Wiegman
 *Will search through file or
 *standard input for a given string
 *
 *Call as:
 * mygrep STRING [FILE...]
 *
 *Based on:
 * Solution to Lab #1 in CS 306, Fall 2008
 * Author: Norman Carver
*/


#include <stdlib.h>
#include <stdio.h>
#include <string.h>
#include <errno.h>
#include <sys/types.h>
#include <sys/stat.h>
#include <fcntl.h>
#include <unistd.h>

#define FILE_BUFFER_SIZE 1024
#define INIT_BUFF_SIZE 50
#define INC_BUFF_SIZE 10

char *get_next_line(int fd);
int get_next_char(int fd);
int grep_open_file (char *search_string,int fd,char *file_pathname, int print_file);

int main(int argc, char *argv[]) {
	char *needle, *file_pathname;
	int i, fd, status = EXIT_SUCCESS;

	if (argc < 2) {
		fprintf(stderr,"Usage: mygrep STRING [FILE...]\n");
		return EXIT_FAILURE;
	}

	needle = argv[1];

	if(argc == 2) {
		file_pathname = "stdin";
		fd = 0;
		grep_open_file(needle,fd,file_pathname,0);
	} else {
		for (i=2; i<argc; i++) {
			file_pathname = argv[i];
			fd = open(file_pathname,O_RDONLY);
			if (errno > 0) {
				fprintf(stderr,"%s: Error opening file %s: %s\n", argv[0], file_pathname, strerror(errno));
				status = EXIT_FAILURE;
			} else {
      			grep_open_file(needle,fd,file_pathname,argc>3);
      			//close(fd);
      		}
		}
	}
  return status;
}

/*open the file or read the standard input and search for our string */
int grep_open_file(char *needle, int fd, char *file_pathname, int print_file) {
	char *haystack;

    while ((haystack = get_next_line(fd)) != NULL) {
    	if (strstr(haystack,needle) != NULL) {
    		if (print_file) {
    			printf("%s:%s\n",file_pathname,haystack);
    		} else {
    			printf("%s\n",haystack);
    		}
    	}
    }

    if (errno > 0) {
    	fprintf(stderr,"Error reading from file %s: %s\n", file_pathname, strerror(errno));
    	return EXIT_FAILURE;
    }

    return EXIT_SUCCESS;
}

/*Get the next line and return it as a valid string */
char *get_next_line(int fd) {
	static char *line_buff;
	char *temp_buff;
	int buff_pos, next_char, chars_left, buff_size;

	buff_size = INIT_BUFF_SIZE;
	chars_left = buff_size;
	if (line_buff != NULL) {
		free(line_buff);
	}
	line_buff = malloc(buff_size);

	buff_pos = 0;

	while ((next_char = get_next_char(fd)) != '\n' && next_char != EOF) {
		if (chars_left > 0) {
			chars_left--;
		} else {
			buff_size = buff_size + INC_BUFF_SIZE;
			temp_buff = line_buff;
			line_buff = realloc(line_buff,buff_size);
			if (line_buff == NULL) {
				line_buff = temp_buff;
			}
		}
		line_buff[buff_pos++] = next_char;
	}
	if (chars_left == 0) {
		buff_size = buff_size + 1;
		temp_buff = line_buff;
		line_buff = realloc(line_buff,buff_size);
		if (line_buff == NULL) {
			line_buff = temp_buff;
		}
	}
	line_buff[buff_pos] = '\0';

	if (next_char == EOF && (buff_pos == 0 || (errno > 0))) {
		free(line_buff);
		return NULL;
	} else {
		return line_buff;
	}
}
/*Get the next character in the file. Make sure to read more than 1 character in per execution*/
int get_next_char(int fd) {
	static char char_buff[FILE_BUFFER_SIZE];
	static int char_count, bytes_left,pos;
	if (bytes_left == 0) {
		char_count = read(fd, char_buff, FILE_BUFFER_SIZE);
		bytes_left = char_count;
		pos = 0;
	}
	if (bytes_left != 0) {
		bytes_left--;
		return char_buff[pos++];
	} else {
		return EOF;
	}
}

Not sure why you get that particular probem, but:

Code:

	if (line_buff == NULL) 
	{
	    line_buff = temp_buff;
	}

is twice in your code, and you REALLY do not want to do that - you WILL be writing over the end of your allocated memory with that - you probably should exit out of the app if you can't get the memory to produce the full string.


Another "interesting" point is that you never "recharge" chars_left when you grow the buffer, so perhaps you are really running out of memory when you reach chars_left = 0.

--
Mats

"double free" usually happens when you free() something that's already been freed. Try wrapping your free calls like this to see if it makes a difference:

Code:

if (strlen(buffer) != 0) free(buffer);

Since a freed char* will always have a length of 0. Using != NULL does not work because a freed pointer is not automatically NULL.

Originally Posted by MK27

Since a freed char* will always have a length of 0.

Unfortunately, that is not always true, since the memory may be freed and yet not overwritten. In fact, if buffer points to freed memory, then what you are doing is using strlen() on a string that does not exist.

Originally Posted by MK27

Using != NULL does not work because a freed pointer is not automatically NULL.

However, using free() on a null pointer is harmless, so it is good practice to set a pointer to NULL after using free() on it, assuming that the pointer does not immediately go out of scope after that (in which case making it a null pointer is useless).

Originally Posted by laserlight

Unfortunately, that is not always true, since the memory may be freed and yet not overwritten. In fact, if buffer points to freed memory, then what you are doing is using strlen() on a string that does not exist.

I think you're wrong:

Code:

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

int main() {
	char *buffer;
	buffer=malloc(12);
	strcpy(buffer,"anything");
	free(buffer);
	printf("%d\n",strlen(buffer));
}

Output: 0
And yet, I never overwrote the freed buffer, and I called strlen on a string that according to you does not exist.

I made my suggestion because it's worked for me everytime I've tried it.

Originally Posted by MK27

I made my suggestion because it's worked for me everytime I've tried it.

Take a look at what C99 says free() should do:

Originally Posted by ISO/IEC 9899:1999 Section 7.20.3.2

The free function causes the space pointed to by ptr to be deallocated, that is, made available for further allocation. If ptr is a null pointer, no action occurs. Otherwise, if the argument does not match a pointer earlier returned by the calloc, malloc, or realloc function, or if the space has been deallocated by a call to free or realloc, the behavior is undefined.

In other words, you are relying on undefined behaviour.

Originally Posted by laserlight

In other words, you are relying on undefined behaviour.

Me or the OP?

I'm not. C99 says "the behavior is undefined" "if the argument does not match a pointer earlier returned by the calloc, malloc, or realloc function". buffer was returned by malloc. It also says "The free function causes the space pointed to by ptr to be deallocated, that is, made available for further allocation." In effect the pointer's space is allocated nothing, which means it will always have a length of 0.

Originally Posted by MK27

It also says "The free function causes the space pointed to by ptr to be deallocated, that is, made available for further allocation." In effect the pointer's space is allocated nothing, which means it will always have a length of 0.

No, it is merely "made available for further allocation". What you are relying on is for the deallocated memory to be zeroed, which is not required by the standard.

Originally Posted by MK27

Me or the OP?

I'm not. C99 says "the behavior is undefined" "if the argument does not match a pointer earlier returned by the calloc, malloc, or realloc function". buffer was returned by malloc. It also says "The free function causes the space pointed to by ptr to be deallocated, that is, made available for further allocation." In effect the pointer's space is allocated nothing, which means it will always have a length of 0.

But once the pointer is "no longer pointing to valid memory", using it as an argument to strlen is completely and utterly undefined. Consider a situation where another thread is using the heap, and allocates the memory you just freed before the call to strlen(), and fills it with some non-zero characters. Now strlen() will definitely be non-zero, and you are now freeing memory belonging to another thread. If the memory has been freed YOU CAN NOT TOUCH IT.

Further, free doesn't make ANY guarantees about what happens to the content of the memory that was freed - it is definitely not certain to give you a string of length 0. In MS C++ using the debug build, it will be filled with some pattern such as 0xDE, which is decidedly non-zero, so it won't make strlen() of it zero! But the most likely scenario is that all of it's content is remaining like it was [again, assuming the memory hasn't been used for something else].

--
Mats

Okay, I will take this to heart. Thanks all.