realloc() invalid pointer

**JayBlay77** · 02-17-2010

So I'm using pthreads to take an unknown number (< unsigned int) of words from a text file, sorting them lexicographically and outputting them to a new file. While this is happening I'm counting the number of lines that are sorted as well as the longest and shortest string that is sorted and storing it in a struct. The idea is for every file that is supposed to be sorted a thread is created.

When I run it with just one file I get a glibc detected realloc(): invalid pointer error and I can't figure out what I'm doing wrong.

Any help would be appreciated. Let me know if you need more info.f

Here's the struct:

Code:

typedef struct _stats_t
{
	char *longest, *shortest;
	unsigned int numlines;
	char *filename;
} stats_t;

Main file:

Code:

#include <stdio.h>              /* Standard buffered input/output        */
#include <stdlib.h>             /* Standard library functions            */
#include <string.h>             /* String operations                     */
#include <pthread.h>			/* Thread related functions				 */
#include <mp2.h>				/* Header specific to this app			 */

int sort(const void *str1, const void *str2)
{
	return strcasecmp((const char*)str2, (const char*)str1);
}

void *newfile(void *threadarg)
{
	stats_t *statstmp;
	FILE *fp, *nfp;
	int count = 0;
	unsigned int i = 0;
	char **stringArray = (char**)malloc(count*sizeof(char*));
	char *mystring = (char*)malloc(256*sizeof(char));
	char *newName = ".sorted";
	

	statstmp = (stats_t*) threadarg;
	
	printf("Filename: %s\n", statstmp->filename);
	
	fp = fopen(statstmp->filename, "r");
	if(fp == NULL)
	{
		fprintf(stderr, "Can't open %s\n", statstmp->filename);
	}
	else
	{
		while(!feof(fp))
		{
			fgets(mystring, 256, fp);
			if(mystring[0] == '\n' || strlen(mystring) == 0)	/* Check for blank lines*/
				continue;
			else
			{
				count++;
				stringArray = (char**)realloc(stringArray,count*sizeof(char*));
				if(stringArray == NULL)
					printf("Error (re)allocating memory\n");
				else
				{
					stringArray[count - 1] = (char*)malloc(strlen(mystring)*sizeof(char) + 2);
					strcpy(stringArray[count - 1], mystring);
					
					statstmp->numlines = count;					
					if(strlen(mystring) > strlen(statstmp->longest))
					{
						statstmp->longest = (char*)realloc(statstmp->longest,
								strlen(mystring)*sizeof(char) + 1);
						strcpy(statstmp->longest, mystring);
					}
					if(strlen(mystring) < strlen(statstmp->shortest) ||
							strlen(statstmp->shortest) == 0)
					{
						statstmp->shortest = (char*)realloc(statstmp->shortest,
								strlen(mystring)*sizeof(char) + 1);
						strcpy(statstmp->shortest, mystring);
					}
				}
			}
		}
		
		qsort(stringArray, statstmp->numlines, sizeof(char*), sort);
		statstmp->filename = (char*)realloc(statstmp->filename,
				strlen(statstmp->filename)*sizeof(char) + 7);
		strcat(statstmp->filename, newName);
		
		nfp = fopen(statstmp->filename, "w");
		if(nfp != NULL)
		{
			for(i = 0; i < statstmp->numlines; i++)
			{
				strcat(stringArray[i], "\n");
				fputs(stringArray[i], nfp);
				free(stringArray[i]);
			}
		}
		
		printf("This worker thread writes %d lines to \"%s\".\n", statstmp->numlines,
				statstmp->filename);
		fclose(nfp);	
		fclose(fp);
		
		free(stringArray);
		free(statstmp->filename);
		free(statstmp->longest);
		free(statstmp->shortest);
		free(statstmp);
	}
		
	pthread_exit(NULL);
}



/* MAIN PROCEDURE SECTION */
int main(int argc, char **argv)
{
	stats_t *stats [argc-1];
	int i, j;
	pthread_t threads[argc - 1];
	
	for(i = 0; i < (argc - 1); ++i)
	{
		printf("Number of args: %i\n", argc);
		stats[i] = (stats_t*)malloc(sizeof(stats_t));
		stats[i]->filename = (char*)malloc((strlen(argv[i+1]) + 1)*sizeof(char));
		stats[i]->filename = (char*)argv[i+1];
		stats[i]->longest = (char*)malloc(sizeof(char));
		stats[i]->longest = "\0";
		stats[i]->shortest = (char*)malloc(sizeof(char));
		stats[i]->shortest = "\0";		
		stats[i]->numlines = 0;
		pthread_create(&threads[i], NULL, newfile, (void*) &(*stats[i]));
	}
	for(j = 0; j < argc - 1; j++)
	{
		pthread_join(threads[j], NULL);
	}
	
	
	pthread_exit(NULL);

	free(stats);

	return 0;
} /* end main() */

And here are the errors:

Code:

[Thread debugging using libthread_db enabled]
Number of args: 2
[New Thread 0x2ab4ac9278a0 (LWP 16614)]
[New Thread 0x41986940 (LWP 16617)]
Filename: output1.txt
*** glibc detected *** /home/cs/mp2: realloc(): invalid pointer: 0x0000000000401241 ***
======= Backtrace: =========
/lib64/libc.so.6(realloc+0x381)[0x33b7c75421]
/home/cs/mp2[0x400c40]
/lib64/libpthread.so.0[0x33b8806617]
/lib64/libc.so.6(clone+0x6d)[0x33b7cd3c2d]
======= Memory map: ========
00400000-00402000 r-xp 00000000 00:97 10706                              /home/cs/mp2 (deleted)
00601000-00602000 rw-p 00001000 00:97 10706                              /home/cs/mp2 (deleted)
04721000-04742000 rw-p 04721000 00:00 0                                  [heap]
40f86000-40f87000 ---p 40f86000 00:00 0 
40f87000-41987000 rw-p 40f87000 00:00 0 
33b7800000-33b781c000 r-xp 00000000 fd:00 2392272                        /lib64/ld-2.5.so
33b7a1b000-33b7a1c000 r--p 0001b000 fd:00 2392272                        /lib64/ld-2.5.so
33b7a1c000-33b7a1d000 rw-p 0001c000 fd:00 2392272                        /lib64/ld-2.5.so
33b7c00000-33b7d4d000 r-xp 00000000 fd:00 2392273                        /lib64/libc-2.5.so
33b7d4d000-33b7f4d000 ---p 0014d000 fd:00 2392273                        /lib64/libc-2.5.so
33b7f4d000-33b7f51000 r--p 0014d000 fd:00 2392273                        /lib64/libc-2.5.so
33b7f51000-33b7f52000 rw-p 00151000 fd:00 2392273                        /lib64/libc-2.5.so
33b7f52000-33b7f57000 rw-p 33b7f52000 00:00 0 
33b8800000-33b8816000 r-xp 00000000 fd:00 2396283                        /lib64/libpthread-2.5.so
33b8816000-33b8a15000 ---p 00016000 fd:00 2396283                        /lib64/libpthread-2.5.so
33b8a15000-33b8a16000 r--p 00015000 fd:00 2396283                        /lib64/libpthread-2.5.so
33b8a16000-33b8a17000 rw-p 00016000 fd:00 2396283                        /lib64/libpthread-2.5.so
33b8a17000-33b8a1b000 rw-p 33b8a17000 00:00 0 
33bc800000-33bc80d000 r-xp 00000000 fd:00 2396286                        /lib64/libgcc_s-4.1.2-20080825.so.1
33bc80d000-33bca0d000 ---p 0000d000 fd:00 2396286                        /lib64/libgcc_s-4.1.2-20080825.so.1
33bca0d000-33bca0e000 rw-p 0000d000 fd:00 2396286                        /lib64/libgcc_s-4.1.2-20080825.so.1
2aaaaaaab000-2aaaaaaac000 rw-p 2aaaaaaab000 00:00 0 
2ab4ac8f6000-2ab4ac8f8000 rw-p 2ab4ac8f6000 00:00 0 
2ab4ac926000-2ab4ac929000 rw-p 2ab4ac926000 00:00 0 
7fff28133000-7fff28148000 rw-p 7ffffffea000 00:00 0                      [stack]
ffffffffff600000-ffffffffffe00000 ---p 00000000 00:00 0                  [vdso]

Program received signal SIGABRT, Aborted.
[Switching to Thread 0x41986940 (LWP 16617)]
0x00000033b7c30265 in raise () from /lib64/libc.so.6

**cas** · 02-17-2010

First, a couple of suggestions:
You should not use while(!feof(fp)). If you do, your program will appear to read the last line of the file twice. Rather, you should do while(fgets(mystring, 256, fp) != NULL). The reason is that feof() is not predictive; it will tell you if the last read function hit EOF, not if the next one will. Since fgets() can tell you when it hits EOF, it make sense to loop on it.

You also needn't cast malloc()/realloc() to the target pointer type, and you needn't multiply anything by sizeof(char), since sizeof(char) is 1. Your code isn't incorrect, but I find it harder to read with such constructs.

Anyway, the real problem lies here:

Code:

stats[i]->filename = (char*)malloc((strlen(argv[i+1]) + 1)*sizeof(char));
stats[i]->filename = (char*)argv[i+1];
stats[i]->longest = (char*)malloc(sizeof(char));
stats[i]->longest = "\0";
stats[i]->shortest = (char*)malloc(sizeof(char));
stats[i]->shortest = "\0";

For each of these pointers, you're allocating space and then immediately overwriting the allocated space. Elsewhere in your code you're (correctly) using strcpy() to fill in allocated space; you should do the same (or similar) here. Otherwise, you wind up doing something like realloc("\0", blah); and of course you cannot resize a string literal like that. I presume this was just a brainfart type of moment, since you're properly using strcpy() elsewhere. If not, I (or someone else) will be happy to explain what's going on.

**JayBlay77** · 02-17-2010

Thank you so much. I cannot believe I did that. I've been working away trying to figure out why it wasn't realloc right and completely missed that.

**Salem** · 02-17-2010

> char **stringArray = (char**)malloc(count*sizeof(char*));
You can start with
char **stringArray = NULL;
and realloc will do what you want.

Also, you should call realloc like this:

Code:

void *temp = realloc ( stringArray, count*sizeof(*stringArray) );
if ( temp != NULL ) {
    stringArray = temp;
} else {
    // error, but at least stringArray is still valid
}

Points to note:
1. if the realloc fails, you don't leak memory (the old memory is valid if realloc fails).
2. note the use of sizeof(*ptr) This means you always get the right size directly from the pointer you're assigning to. There is NO need to go back to the declaration to figure out the type.

> and you needn't multiply anything by sizeof(char), since sizeof(char) is 1.
True, but in my experience, when people start doing "optional" programming, that is when hard to spot bugs creep in (especially when "optional" becomes "compulsory" and there isn't an error message telling you).

The
p = malloc ( n * sizeof *p )
construct works all the time in all cases (save for arithmetic overflow, but that's a whole other story).

Thread: realloc() invalid pointer

Thread Tools

Search Thread

Display

realloc() invalid pointer

Similar Threads

Why does C need pointer conversion

Build an array of strings dynamically

Direct3D problem

realloc and pointer conversion from sub to main function

Can an invalid pointer clobber the new operator?