realloc() invalid pointer

This is a discussion on realloc() invalid pointer within the C Programming forums, part of the General Programming Boards category; So I'm using pthreads to take an unknown number (< unsigned int) of words from a text file, sorting them ...

  1. #1
    Registered User
    Join Date
    Mar 2009
    Posts
    3

    Angry realloc() invalid pointer

    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

  2. #2
    cas
    cas is offline
    Registered User
    Join Date
    Sep 2007
    Posts
    993
    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.

  3. #3
    Registered User
    Join Date
    Mar 2009
    Posts
    3
    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.

  4. #4
    and the hat of wrongness Salem's Avatar
    Join Date
    Aug 2001
    Location
    The edge of the known universe
    Posts
    32,761
    > 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).
    If you dance barefoot on the broken glass of undefined behaviour, you've got to expect the occasional cut.
    If at first you don't succeed, try writing your phone number on the exam paper.
    I support http://www.ukip.org/ as the first necessary step to a free Europe.

Popular pages Recent additions subscribe to a feed

Similar Threads

  1. Why does C need pointer conversion
    By password636 in forum C Programming
    Replies: 2
    Last Post: 04-10-2009, 08:33 AM
  2. Build an array of strings dynamically
    By Nazgulled in forum C Programming
    Replies: 29
    Last Post: 04-07-2007, 10:35 PM
  3. Direct3D problem
    By cboard_member in forum Game Programming
    Replies: 10
    Last Post: 04-09-2006, 04:36 AM
  4. Replies: 2
    Last Post: 02-07-2002, 09:39 AM
  5. Can an invalid pointer clobber the new operator?
    By Unregistered in forum C++ Programming
    Replies: 3
    Last Post: 09-14-2001, 04:42 AM

1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21