Dynamic char array - memory leak?

[bh7322]

Hi All.
I'm new to the forum. I'm having some issues with my program. I seem to be accessing memory that I shouldn't be (surprise, surprise!), but I can't see the problem. Keen eyes are welcome! Any help appreciated.

Sample input: Enter word for retrieval: the Sample output: You entered: ' Uthe' The program in its entirety counts the number of words, puts each word into a linked list and then searches the list from a text file input taken from the 'command prompt'.

/*dynamically expands array and inserts new letter at the end*/
char * makeword( char c, char *wrd, int wrd_len ) {
  char *ch = &c;
  /*alloc memory enough for existing word, new letter and '\0'*/
  /*adapted from http://www.cplusplus.com/reference/clibrary/cstdlib/realloc.html*/
  wrd = (char*)realloc(wrd, (wrd_len+1)*sizeof(char));
  if( wrd == NULL ) {
    perror("realloc returned NULL. Unable to allocate memory.") ;
    exit (-1) ;
  }
  /*concatenate existing word with new letter*/
  strncat( wrd, ch, 1 ) ;
  return wrd ;  
} 

/*takes file pointer, reads (letter by letter) a word into a 'dynamic' char array*/
char * wordify( FILE * ifp, char *wrd ){
  unsigned int c, prev_char = 0, wrd_len = 0 ;
  
  /*make words until EOF is read*/
  do{		
    c = getc( ifp ) ; 
    
    /*check if c is a letter. getc returns
    EOF as an unsigned int, so we must check for it aswell*/    
    if( ( isalnum( c ) || isvalid( c, prev_char, ifp ) ) && c != EOF ) {
      wrd = makeword ( tolower( c ), wrd, wrd_len ) ;
      prev_char = 1 ;
	
      /*increment counter when this letter is non-alpha but previous was alpha*/
    } else if( !isalnum( c ) && c != EOF && prev_char == 1 ) {
      prev_char = 0 ;
      return wrd ;
    }

  }while( c != EOF ) ;
  
  return wrd ;
}

void search() {
  char *findthis = NULL, repeat ;
  
  do{ 
    printf( "Enter word for retrieval: " ) ;
    findthis = wordify(stdin, findthis);
    printf( "You entered: %s", findthis ) ;  /*prints stdin with garbage infront*/

    findthis = NULL;
    
    printf( "\n\nSearch again? [y/n]:  " ) ;
    repeat = getchar() ;    
    
  }while( 'y' == repeat || 'Y' == repeat ) ;
}

[Salem]

> wrd = (char*)realloc(wrd, (wrd_len+1)*sizeof(char))
1. was it really worth dropping a single letter?
2. wrd_len never gets incremented.
3. you're assuming the memory returned by realloc is zero filled - it isn't.
4. your use of realloc is unsafe, but more on that later no doubt.

[bh7322]

Apologies. That should have been:

if( ( isalnum( c ) || isvalid( c, prev_char, ifp ) ) && c != EOF ) {
      wrd_len++ ; /*NB. incremented before passed to makeword*/
      wrd = makeword ( tolower( c ), wrd, wrd_len ) ;

I'm afraid I don't understand the majority of what you've said Salem. I decided I couldn't get the correct parsing of the text by reading it straight into an array, and I thought a linked list of chars was counter-intuitive.

3. How does this assumption affect the outcome. My thought would be that I assign the chars to the memory and therefore the initial value is irrelevant?

[Salem]

> My thought would be that I assign the chars to the memory and therefore the initial value is irrelevant?
Two things about strncat
1. It attempts to find a \0, then appends characters. If memory is filled with garbage (which it is), then you're appending the data in an unknown place.
2. strncat doesn't always append a \0, so you lose again.

You already have an index, so use it.
wrd[wrdlen] = ch;
wrd[wrdlen+1] = '\0';
simple, works.

[bh7322]

Works a charm, thanks for the insight!