I'm trying to get myself acquainted with memory allocation and deallocation while writing a program that will actually be usefull for me.

The program is a simple datareader, that tries to read a matrix from file.

The "main" program which works is
'int get_lex(char *in,char *delims,char ***returnvals)'
This will essential return a argv style list of the tokens through 3.parameter.

like

char **line=NULL;
int numToks = get_lex("small.txt"," \t",&line)
for(int i=0;i<numtoks;i++) 
    printf("%i %s",i,line[i]);

The innerworkings of this is a single linked list of char*, that rolls back to a normal array as soon as the cstring is tokenized.
This works.

Now as I loop through the lines of the files I've created another singlelinked list which will have the tokenized argv style string from get_lex, and then a pointer to the next row.

The program works as is!, but I can't nail down a memoryleak that shows up when using valgrind.
A sample file: small.txt would be

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

and the program can be run as
'./a.out small.txt'


Heres the full program thanks in advance,
btw I know this can be done easily with std::string and vector or some booost::tokenizer.
But I just want to know how I can fix this faulty program
I know the code is huge but the problemeatic functions are get_lex, readmatrix and friends.

valgrind complains at line 151 the strdup. But I don't know where I should deallocate.


Thanks again

#include <fstream>
#include <cstring>
#include <cstdlib>
#include <iostream>
#define MAX_ELEMS_PER_LINE 1000


typedef struct  {
  int x;
  int y;
  int** matrix;
  void info(const char *name){
    printf("iMatrix:%s dims= (%d,%d) \n",name,x,y);
  }

  void print(const char *st,const char *file){
    if(file==NULL){
      if(st!=NULL)
	printf("\niMatrix dims: %s = (%d,%d)\n",st,x,y);
      for (int i=0;i<x;i++){
	for (int j=0;j<y;j++)
	  printf("%d ",matrix[i][j] );
	printf("\n");
      }
    } else{
      FILE *pFile;
      pFile = fopen(file,"w");
      if(st!=NULL)

	fprintf(pFile,"\niMatrix dims of: %s = (%d,%d)\n",st,x,y);
      for (int i=0;i<x;i++){
	for (int j=0;j<y;j++)
	  fprintf(pFile,"%d ",matrix[i][j]);
	fprintf(pFile,"\n");
      }
      fclose(pFile);
    }
  }
}iMatrix ;


void killIntMatrix(int **var){
  delete [] *var;
  delete [] var;
}



void killMatrix(iMatrix *var){
  killIntMatrix(var->matrix);
  delete var;
}




#define _fillup_ 1

iMatrix *allocIntMatrix(int x, int y){
  try{
    iMatrix *tmp = new iMatrix();
    int **ppi = new int*[x];
    int *curPtr = new int [x * y];
    
    for( int i = 0; i < x; ++i) {
      *(ppi + i) = curPtr;
      curPtr += y;
    }
#if _fillup_
    for (int i=0;i<x;i++)
      for(int n=0;n<y;n++)
	ppi[i][n]=0;
#endif
    tmp->matrix=ppi;
    tmp->x=x;
    tmp->y=y;
    return tmp;
  }catch(std::exception & e){
    printf("\t-> Tried to allocate: %.3f gig memory\n",(float)sizeof(int)*x*y/1000000000);
    printf("\t-> Allocation failed, likely cause: need more memory, will exit.\n");
    exit(0);
  }
}




//single linked list definition
typedef struct node_t{
  char *data;
  struct node_t *next;
}node;

typedef struct row_t{
  char **data;
  struct row_t *next;
}row;



//single linked list node memory allocation
node *alloc(char *ary){
  node *retVal = new (node);
  retVal->data = ary;
  retVal->next = NULL;
  return retVal;
}

row *allocRow(char **ary){
  row *retVal = new (row);
  retVal->data = (ary);
  retVal->next = NULL;
  return retVal;
}
void deleteRow(row *tmp){
  char **ary = tmp->data;
  delete [] ary;
  delete tmp;
}


int get_lex(char *str,const char *delim,char ***tokens){
  int len =0;
  char *token = 0;
  node *head,*current;


  //1. tokenize the string to a linked list
  token = strtok(str,delim);
  if(!token)//catch an empty line
    return 0;
  head = alloc(token);
  current=head;

  len++;
  while ( token != NULL ){
    token = strtok(0,delim);
    if(!token)//catch spurious NULL\s (multi delims...)
      continue;
    current->next = alloc(token);
    current = current->next;
    len++;
  }
  if(!tokens)
    printf("warning, in you string splitter you have supplied a non null cstring pointer");
  //2. populate and array and input the nodes from the linked list.
  (*tokens)  = new char*[len];
  int i=0;
  current=head;
  while(current){
    (*tokens)[i++] =strdup( current->data);
    head= current->next;
    delete current;
    current=head;
  }

  return len;
}




void print(char** tmp,int len){
  int i;
  for(i=0;i<len;i++)
    printf("%d: %s\n",i,tmp[i]);

}


iMatrix *readmatrix(char *filename){
  
  const int SIZE = MAX_ELEMS_PER_LINE;
  char buffer[SIZE];
  
  std::ifstream pFile (filename,std::ios::in);
   if(!pFile){
    std::cout <<"Problems opening file" <<filename<<std::endl;
    exit(0);
  }
   
   row *head,*current;
   int numberOfColumns;
   int numRow=0;
   char *scpy;
   while(!pFile.eof()){
     pFile.getline(buffer,SIZE);
     scpy =buffer;// strdup(buffer);     
     char **line=NULL;
     if(numRow==0){

       numberOfColumns = get_lex(scpy," \t",&line);
       
       head = allocRow(line);
       current = head;
       numRow++;
     }else{

       int tmp =  get_lex(scpy," \t",&line);
       if(tmp==0){
	 continue; 
       }
       if(tmp!=numberOfColumns){
	 printf("\t-> Probs at row: %d columnlength mismatch %d vs. %d\n",numRow,numberOfColumns,tmp);
	 exit(0);
       }
       current->next = allocRow(line);
       current = current->next;
       numRow++;
     }

   }
   printf("dims are:%d %d\n",numRow,numberOfColumns);
   
   //now lets input our rows of chars into the matrix
   iMatrix *retVal = allocIntMatrix(numRow,numberOfColumns);
   int i=0;//the rows

   while(head!=NULL){
     for(int j=0;j<numberOfColumns ; j ++)
       retVal->matrix[i][j] = atoi(head->data[j]);
     current = head->next;
     deleteRow(head);
     head = current;
     i++;
   }
   return retVal;
}


int main(int argc, char **argv){

  iMatrix *var = readmatrix(argv[1]);
  var->print(NULL,NULL);
  killMatrix(var);
  return 0;
}

> valgrind complains at line 151 the strdup. But I don't know where I should deallocate.
In whoever calls get_lex(), just after they've finished with the tokens would be good.

Hi thanks for your reply,
but I don't know the syntax for clearing something that has been allocted like

char ***tokens;
(*tokens) = new char*[len];
for(int i=0 ; i<len ; i++)
  (*tokens)[i] =strdup("cstring");

I solved it,
strdup uses malloc,
so whenever you use a these cstring functions,
you should use free and not delete.

Well, your program is a chaotic mix of C and C++.

You might want to work on that for a while, before you get too comfortable with the lazy "it works today" approach.

Start by replacing all printf() with std::cout and all char arrays (and char pointers) with std::string.

Well, your program is a chaotic mix of C and C++.

You might want to work on that for a while, before you get too comfortable with the lazy "it works today" approach.

Start by replacing all printf() with std::cout and all char arrays (and char pointers) with std::string.

Hi,
Thanks for you reply,
apparently I don't get notified when people reply anymore.
So sorry for the delayed response.

I was using std::string,
But it is ridicously slow compared to char*.

I'm reading and parsing huge textfiles, on the mulitigig scale, and many of these.

I tried the std::vector<std::string> approach, and it was very very slow. Each file took aprox 10minutes,
while the single linked list with char arrays took less than 2 minutes.


I agree that it's not a good thing to intermix c/c++ generally

But once in awhile if you know what you are doing, I would deem it ok.

Just as a goto statement makes perfect sense in jumptable.

And not even the c++ devs follows this apartheid approach of std::string vs char*.

The libraries are full of char* function prototypes.
like
'fstream::open(const char*)'

And just the lack of a c++ std::string tokenizer justifies char* in a c++ program.

thanks for your reply

This question has already been answered. Start a new discussion instead.