Strncat and Realloc - pointer issues

**bhenderson** · 08-16-2009

Hi all,

This is a university project that compares linked list and hash table efficiencies (eventually). The problem here I believe is my use of the pointers. Can anyone give me some advice on why I'm going wrong? It seems to have problems when it encounters 'strncat' in the apendchar function, but I can't see why. I'm supposed to be writing to C90 standard. I've included the compiler warnings at the bottom.

Thanks in advance!

Code:

typedef enum { FALSE, TRUE } boolean ;

#include "stdio.h"
#include "stdlib.h"		/*calloc*/
#include "ctype.h"		/*isalnum*/
#include "string.h"		/*string functions*/
/*#include "time.h"		/*Execution time*/ */
#include "grammar.c"		/*additional grammar checks for - and ' */


/*Places addtional letters at the end of the current word*/
char * apendchar( unsigned int *ch, char *wrd, int *wrd_len ) {
  wrd = (char*)realloc(*wrd, (*wrd_len+1)*sizeof(char));
  if( wrd == NULL ) {
    perror("realloc returned NULL. Unable to allocate memory.") ;
    exit (-1) ;
  }
  strncat( wrd, *ch, 1 ) ;
  return wrd ;  
}


/*Grabs a single word at a time from the input file*/
char * fetchword( FILE * ifp, char *wrd, int *wrd_count ){
  unsigned int c ;
  int wrd_len = 0 ;
  boolean prev_char = FALSE ;       /*remember validity of previous letter read for grammar checking*/
  
  do{		/*loop until EOF*/
    c = getc( ifp ) ;    
    if( isvalid( &c, ifp, &prev_char ) ) {
      wrd_len++ ;
      c = tolower( c ) ;
      *wrd = apendchar ( &c, wrd, &wrd_len ) ;
      prev_char = TRUE ;
    }
  
    /*increment wrd_count at the end of each word*/
    if( !isvalid( &c, ifp, &prev_char ) && c != EOF ) {
      *wrd_count++ ;
      return wrd ;
    }

  }while( c != EOF ) ;
  
  /*Mustn't forget the EOF case*/
  return wrd ;
}

/*takes file from command prompt*/
int main( int argc, char *argv[] ) {
  FILE *ifp = NULL ;		/*in-file pointer*/
  char *wrd ;
  int wrd_count = 0 ;
  
  /*read file from command prompt. fopen returns NULL if error. */
  ifp = fopen(argv[1], "r");	
  if( ifp == NULL ) {
    perror("fopen returned NULL. Check spelling or file location.") ;
    exit (-1) ;
  }
  do{                /*loop until EOF*/
    wrd = NULL ;
    wrd = fetchword( ifp, &wrd, &wrd_count ) ;
    /*do other stuff*/
    
  } while ( wrd != NULL ) ;
  
  printf( "\n%d word(s) read.\n", wrd_count ) ;
  
  fclose( ifp ) ;
  return 0 ;
}

Code:

boolean isvalid ( unsigned int *test_char, FILE * ifp, boolean *prev_char ) {
  unsigned int test;

/*All alpha-numeric characters are valid input*/
  if ( isalnum( *test_char ) && *test_char != EOF )
    return TRUE ;

/*return true only if - and ' are part of an abbreviation such as: test-case or they'll*/
  if ( *prev_char && ( *test_char == '-' || *test_char == '\'') && *test_char != EOF ) {
    test = getc( ifp ) ; 
    if ( isalnum( test ) ) {
      ungetc( test, ifp ) ;
      return TRUE ;
    } else {
      return FALSE ;
    }
  }
  return FALSE ;
}

GCC Warnings:

Code:

In function ‘apendchar’:
40: warning: passing argument 1 of ‘realloc’ makes pointer from integer without a cast
45: warning: passing argument 2 of ‘strncat’ makes pointer from integer without a cast
In function ‘fetchword’:
59: warning: assignment makes integer from pointer without a cast
65: warning: value computed is not used (wrd_count)
In function ‘main’:
89: warning: passing argument 2 of ‘fetchword’ from incompatible pointer type

**Elysia** · 08-16-2009

Right off the bat, I see that you are trying to pass a pointer to your char* pointer in main to fetchword. Can you spot the problem? You are passing the address of a pointer, while the function expects a pointer to char, not char*. Your function must accept a pointer to char* (char**).
This bug is in your appendchar too. Because you said it accepts a char*, you dereference that to get a char, which causes a warning (really an error).
The strncat call is absolutely wrong. It takes two pointers to char: the destination and the source, and finally the number of chars to copy maximum. What are you passing to it?
You are including a .c file. NEVER do this. All source files MUST be compiled separately.
And last but not least, remove the cast from malloc.

**tabstop** · 08-16-2009

Same deal with apendword -- if you intend to change the value of wrd, you must pass it in by pointer. And passing a char * by pointer means you are passing a char **.

**bhenderson** · 08-16-2009

So I'm thinking from the heavy hints that I need to change the code to something more like this. I get the feeling though that there's still an error in my logic.

Code:

char * apendchar( unsigned int *ch, char **wrd, int *wrd_len ) {
  wrd = realloc(*wrd, (*wrd_len+1)*sizeof(char)); /* *wrd is a char* as required by realloc. */
  strncat( wrd, *ch, 1 ) ; /*no idea at the minute*/
  return wrd ;  
}


char * fetchword( FILE * ifp, char **wrd, int *wrd_count ){
  unsigned int c ;
  int wrd_len = 0 ;

      *wrd = apendchar ( *c, wrd, *wrd_len ) ; /*the actual value of wrd 
(dereferenced) is given by *wrd. I pass a pointer to c, by using *c, to the 
next function. Since wrd is now a double pointer in this function, the next 
function must accept a char**. */

      return wrd;
}


int main( ) {
  char *wrd ;
    
    wrd = NULL ;
    *wrd = fetchword( ifp, wrd, *wrd_count ) ; 
   /*where wrd is now a pointer to a char array*/
}

**Elysia** · 08-16-2009

You don't quite get the hang of pointers.
So you want to pass a variable to another function so it can modify it. How do we do that? We pass a pointer normally. So we want to modify an int, we pass a pointer to that int, ie int*:

Code:

void foo(int * p) { *p = 5; }

int x;
foo(&x);

The pointer syntax is easy: everything to the left of the * is what the pointer points to - in this case int.
Let's modify it to work with a pointer instead. We want to pass a pointer to another function to modify it. This works exactly the same as with this example. We only need to modify the type the function accepts.

Code:

void foo(int* * p) { *p = malloc(sizeof(int)); }

int* x;
foo(&x);
free(x);

Difficult? I don't think so.
With this in mind, can you correct the code?

As for what's wrong with strncat - let's take a look at the strncat prototype:
char* strncat(char* dst, const char* src, int CopyMax);

What parameters are you passing to strncat at the moment?

**bhenderson** · 08-16-2009

Ok, thanks Elysia, I feel happier with passing a pointer from one function and then back, but does that also apply when passing it multiple times before returning it? For example, I pass 'wrd' from main into fetchword and then into apendchar before it returns. Also in this instance what should I return? I imagine that I need to dereference the returning value? i.e. return *wrd on its way back.

As I understand it realloc creates a pointer to the memory allocated, so I'm thinking that when I assign that to 'wrd', 'wrd' needs to be derefenced until it's just a single pointer?

For strncat, I want to be feeding it two char arrays or char*. So in my case I think that 'ch' is already a char* so that can be fed in as it is. But I get confused (see my previous point) whether 'wrd' can be fed in as a char** or needs dereferencing by one level into a char*.

On another note, what is the generally the prefered way for converting the unsigned int into a char? I was changing the typecast inbetween functions, but I get a warning when I do that.

Modified code:

Code:

typedef enum { FALSE, TRUE } boolean ;

#include "stdio.h"
#include "stdlib.h"		/*calloc*/
#include "ctype.h"		/*isalnum*/
#include "string.h"		/*string functions*/
#include "time.h"		/*Execution time*/
#include "grammar.c"     /* TO CHANGE...additional grammar checks for - and ' */

char * apendchar( char *ch, char **wrd, int *wrd_len ) {
  *wrd = realloc( wrd, ((*wrd_len)+1)*sizeof(char) ) ;  /*realloc gives back a 
single pointer, therefore I need to dereference wrd by one level*/
  if( wrd == NULL ) {
    perror("realloc returned NULL. Unable to allocate memory.") ;
    exit (-1) ;
  }
  strncat( *wrd, ch, 1 ) ;     /*ch is fine because it is a char*, but wrd is a char** 
so I dereference it by one level into a char* */
  return wrd ;           /*unsure*/
}

char * fetchword( FILE *ifp, char **wrd, int *wrd_count ) {
  unsigned int c ;
  int wrd_len = 0 ;
  boolean prev_char = FALSE ;
  
  do{		
    c = getc( ifp ) ;    
    if( isvalid( &c, ifp, &prev_char ) ) {
      wrd_len++ ;
      c = tolower( c ) ;
      wrd = apendchar( &c, &wrd, &wrd_len ) ;
      prev_char = TRUE ;
    }
  
    /*increment wrd_count at the end of each word*/
    if( !isvalid( &c, ifp, &prev_char ) && c != EOF ) {
      *wrd_count++ ;
      return *wrd ;          /*unsure*/
    }

  }while( c != EOF ) ;     /*Mustn't forget the EOF case*/
  
  return *wrd ;             /*unsure*/
}

int main( int argc, char *argv[] ) {
  FILE *ifp = NULL ;		/*in-file pointer*/
  char *wrd ;
  int wrd_count = 0 ;
  
  /*read file from command prompt. fopen returns NULL if error. */
  ifp = fopen(argv[1], "r");	
  if( ifp == NULL ) {
    perror("fopen returned NULL. Check spelling or file location.") ;
    exit (-1) ;
  }
  do{
    wrd = NULL ;
    wrd = fetchword( ifp, &wrd, &wrd_count ) ;  
  } while ( wrd != NULL ) ;
  
  printf( "\n%d word(s) read.\n", wrd_count ) ;
  
  fclose( ifp ) ;
  return 0 ;
}

**Elysia** · 08-16-2009

Originally Posted by bhenderson

Ok, thanks Elysia, I feel happier with passing a pointer from one function and then back, but does that also apply when passing it multiple times before returning it? For example, I pass 'wrd' from main into fetchword and then into apendchar before it returns. Also in this instance what should I return? I imagine that I need to dereference the returning value? i.e. return *wrd on its way back.

You seem to have the wrong mindset of pointers. Don't think "if I do this, I get char*" and "if I do this, I get char**".
Think instead: what do I need? Example:

Code:

void foo(int* * p)
{
    *p = malloc(sizeof(int));
}

int main()
{
    int* p;
    foo(&p);
    free(p);
}

First, let's think what foo requires. It needs to store the address returned from malloc in a pointer, yes? And the caller should pass in the pointer to which we should store the result. So we need a pointer to that pointer. Or we need a pointer to that variable, might be easier.
So how do we store it, then? We need to store it in the variable passed in, so we simply dereference the pointer. Simple stuff, no?

If it helps you keep track of the original variable type, do as I do:
Type * Name. Example: if the variable we want to pass in is of type int*, then we can type int* * Name.

Have you worked with pointers (single) before? You should know that a pointer is a variable. And it stores a memory address. So as long as you don't change its value (the address it contains), you can forward it and return it however many times you want. Just as you would do with, say, an int. Because that's basically what it is. A variable storing an integer value.

As I understand it realloc creates a pointer to the memory allocated, so I'm thinking that when I assign that to 'wrd', 'wrd' needs to be derefenced until it's just a single pointer?

Realloc returns a memory address to the block of memory that contains your desired size. So you need to store it in the original char* variable.

For strncat, I want to be feeding it two char arrays or char*. So in my case I think that 'ch' is already a char* so that can be fed in as it is. But I get confused (see my previous point) whether 'wrd' can be fed in as a char** or needs dereferencing by one level into a char*.

"wrd" is a pointer to your string. Don't forget that.

On another note, what is the generally the prefered way for converting the unsigned int into a char? I was changing the typecast inbetween functions, but I get a warning when I do that.

The best way is avoiding conversion in the first place. If you can, keep it char. Otherwise you may assign it to a char, but use a cast. That silences any warnings. Just remember that you may lose data in the process (because a char cannot hold as much data as an int).

There is also additional concern. You seem to treat a single char as a string. That's wrong.
A string in C must end with a '\0'. So an empty string is one char. But a string of length 1 is 2 chars. fgetc returns a single char. But to make this into a string, you need to put it into an array and the element right next to the char must be '\0', otherwise string functions will not work (such as strncat).

Thread: Strncat and Realloc - pointer issues

Thread Tools

Search Thread

Display

Strncat and Realloc - pointer issues