Review and criticism appreciated!

This is a discussion on Review and criticism appreciated! within the C Programming forums, part of the General Programming Boards category; Was writing a lexer(lexical analyzer) function and well I hadn't messed with dynamic memory allocation before was like the last ...

  1. #1
    Sys.os_type="Unix";;
    Join Date
    Aug 2005
    Posts
    52

    Review and criticism appreciated!

    Was writing a lexer(lexical analyzer) function and well I hadn't messed with dynamic memory allocation before was like the last thing left in my k&r book.
    So anyway criticize away! Just want to figure out how I'm doing so far with teaching myself C.

    Code:
    int lexer(char s[], int *ptr, char **ptrarray)
    {
    	int i, w=0, n;
    	int rows=0, cols=0;
    	char *buffptr;
    	int *temp;
    	
    	temp = ptr;
    	while(*temp != 0)
    	{
    		rows++;
    		i = *temp;
    		if(i>cols)
    			cols=i;
    		++temp;
    	}
    	
    	for(i=0; i<rows; i++)
    		if( (ptrarray[i] = malloc((cols+1) * sizeof(char *))) == NULL)
    		{
    			printf("Memory Allocation Failed\n");
    			while(i>0)
    			{
    				free(ptrarray[i]);
    				i--;
    			}
    			exit(-1);
    		}
    	
    	buffptr = s;
    	while( (n = *ptr) != 0 && n <2048)
    	{
    		strcpy(ptrarray[w], buffptr);
    		
    		for(i=0; i<n; i++)
    			++buffptr;
    
    		++ptr;
    		w++;
    	}
    	return rows;
    }
    **Explanation of function**
    Basically takes a string and an int array where each element of the int array is loaded with different lengths of each token you want to break apart a string. I used a pointer array to set a pointer to the beginning of each token in the string.

    Quick and dirty example use..

    Code:
    #include <string.h>
    #include <stdio.h>
    #include <stdlib.h>
    
    int lexer(char s[], int *ptr, char **ptrarray);
    
    int main(void)
    {
    	char buffer[200] = "This is my string test yeah";
    	char testbuff[200];
    	char **ptrarray;
    	char *temp;
    	int array[20];
    	int i, w, rows;
    
    	for(i=0; i<20; i++)
    		array[i] = 0;
    
    	array[0] = 5;
    	array[1] = 3;
    	array[2] = 3;
    	array[3] = 7;
    	array[4] = 5;
    	array[5] = 4;
    
    
    	rows = lexer(buffer, array, ptrarray);
    
    	w=0;
    	while(array[w] != 0)
    	{
    		temp = ptrarray[w];
    		for(i=0; i<array[w]; i++)
    
    		{
    			testbuff[i] = *temp;
    			++temp;
    		}
    		testbuff[i++] = '\0';
    		printf("%s\n", testbuff);
    		w++;
    	}
    
    	for(i=0; i<rows; i++)
    		free(ptrarray[i]);
    	return 0;
    }
    int lexer(char s[], int *ptr, char **ptrarray)
    {
    	int i, w=0, n;
    	int rows=0, cols=0;
    	char *buffptr;
    	int *temp;
    	
    	temp = ptr;
    	while(*temp != 0)
    	{
    		rows++;
    		i = *temp;
    		if(i>cols)
    			cols=i;
    		++temp;
    	}
    	
    	for(i=0; i<rows; i++)
    		if( (ptrarray[i] = malloc((cols+1) * sizeof(char *))) == NULL)
    		{
    			printf("Memory Allocation Failed\n");
    			while(i>0)
    			{
    				free(ptrarray[i]);
    				i--;
    			}
    			exit(-1);
    		}
    	
    	buffptr = s;
    	while( (n = *ptr) != 0 && n <2048)
    	{
    		strcpy(ptrarray[w], buffptr);
    		
    		for(i=0; i<n; i++)
    			++buffptr;
    
    		++ptr;
    		w++;
    	}
    	return rows;
    }
    I know one thing I need to get in the habit of is commenting my code while I write it, I usually end up going back through and commenting it... bleh.

  2. #2
    Frequently Quite Prolix dwks's Avatar
    Join Date
    Apr 2005
    Location
    Canada
    Posts
    8,048
    Code:
    for(i=0; i<n; i++)
        ++buffptr;
    ->
    Code:
    buffptr += n;
    dwk

    Seek and ye shall find. quaere et invenies.

    "Simplicity does not precede complexity, but follows it." -- Alan Perlis
    "Testing can only prove the presence of bugs, not their absence." -- Edsger Dijkstra
    "The only real mistake is the one from which we learn nothing." -- John Powell


    Other boards: DaniWeb, TPS
    Unofficial Wiki FAQ: cpwiki.sf.net

    My website: http://dwks.theprogrammingsite.com/
    Projects: codeform, xuni, atlantis, nort, etc.

  3. #3
    Frequently Quite Prolix dwks's Avatar
    Join Date
    Apr 2005
    Location
    Canada
    Posts
    8,048
    Some of your while loops would probably be better as for loops.

    Code:
    while( (n = *ptr) != 0 && n <2048)
    Is that code right? Or should it involve ptr somehow?
    dwk

    Seek and ye shall find. quaere et invenies.

    "Simplicity does not precede complexity, but follows it." -- Alan Perlis
    "Testing can only prove the presence of bugs, not their absence." -- Edsger Dijkstra
    "The only real mistake is the one from which we learn nothing." -- John Powell


    Other boards: DaniWeb, TPS
    Unofficial Wiki FAQ: cpwiki.sf.net

    My website: http://dwks.theprogrammingsite.com/
    Projects: codeform, xuni, atlantis, nort, etc.

  4. #4
    Sys.os_type="Unix";;
    Join Date
    Aug 2005
    Posts
    52
    Quote Originally Posted by dwks
    Code:
    for(i=0; i<n; i++)
        ++buffptr;
    ->
    Code:
    buffptr += n;
    Ah! Didn't think of that, thanks dwks

  5. #5
    Sys.os_type="Unix";;
    Join Date
    Aug 2005
    Posts
    52
    Quote Originally Posted by dwks
    Some of your while loops would probably be better as for loops.

    Code:
    while( (n = *ptr) != 0 && n <2048)
    Is that code right? Or should it involve ptr somehow?

    That was some old code that slipped through my redesign I'll take out the

    && n<2048

  6. #6
    Frequently Quite Prolix dwks's Avatar
    Join Date
    Apr 2005
    Location
    Canada
    Posts
    8,048
    Code:
    	while(array[w] != 0)
    	{
    		temp = ptrarray[w];
    		for(i=0; i<array[w]; i++)
    
    		{
    			testbuff[i] = *temp;
    			++temp;
    		}
    		testbuff[i++] = '\0';
    		printf("%s\n", testbuff);
    		w++;
    	}
    
    	for(i=0; i<rows; i++)
    		free(ptrarray[i]);
    	return 0;
    }
    You don't need that ++.
    dwk

    Seek and ye shall find. quaere et invenies.

    "Simplicity does not precede complexity, but follows it." -- Alan Perlis
    "Testing can only prove the presence of bugs, not their absence." -- Edsger Dijkstra
    "The only real mistake is the one from which we learn nothing." -- John Powell


    Other boards: DaniWeb, TPS
    Unofficial Wiki FAQ: cpwiki.sf.net

    My website: http://dwks.theprogrammingsite.com/
    Projects: codeform, xuni, atlantis, nort, etc.

  7. #7
    Ultraviolence Connoisseur
    Join Date
    Mar 2004
    Posts
    504
    Code:
    #include <string.h>
    #include <stdio.h>
    #include <stdlib.h>
    
    int lexer(char s[], int *ptr, char **ptrarray);
    
    int main(void)
    {
    	char buffer[200] = "This is my string test yeah";
    	char testbuff[200];
    	char **ptrarray;
    	char *temp;
    	int array[20];
    	int i, w, rows;
    
    	for(i=0; i<20; i++)
    		array[i] = 0;
    
    	array[0] = 5;
    	array[1] = 3;
    	array[2] = 3;
    	array[3] = 7;
    	array[4] = 5;
    	array[5] = 4;
    
    
    	rows = lexer(buffer, array, ptrarray);
    
    
    ....
    I know this is just an example you threw together, but its possible to change all the initialization you have for "array[20]" to simply: int array[20] = {5,3,3,7,5,4,0} (will set the rest of the elements to 0.).

  8. #8
    Sys.os_type="Unix";;
    Join Date
    Aug 2005
    Posts
    52
    I know you can load an array like that but didn't know it would fill in the remaining elements of the array with the last you specify.
    Thanks

    Thanks for all the criticisms so far

  9. #9
    Ultraviolence Connoisseur
    Join Date
    Mar 2004
    Posts
    504
    Quote Originally Posted by Sysop_fb
    I know you can load an array like that but didn't know it would fill in the remaining elements of the array with the last you specify.
    Thanks

    Thanks for all the criticisms so far
    Yeah, in fact you dont even need that extra "0" i put in the last element, it will fill the rest of the array with zeros if you only initialize one element, I just put it there for clearence.

  10. #10
    Sys.os_type="Unix";;
    Join Date
    Aug 2005
    Posts
    52
    Cool

    Also is this a good thing to do when mallocing an array of pointers? If malloc hits NULL after mallocing some already then freeing the ones you already malloced?

    Code:
    	for(i=0; i<rows; i++)
    		if( (ptrarray[i] = malloc((cols+1) * sizeof(char *))) == NULL)
    		{
    			printf("Memory Allocation Failed\n");
    			while(i>-1)
    			{
    				free(ptrarray[i]);
    				i--;
    			}
    			exit(-1);
    		}
    I also changed that from my original instead of while(i>0) while(i>-1) that way it doesn't miss freeing ptrarray[0].

Popular pages Recent additions subscribe to a feed

Similar Threads

  1. Please review my first tutorial
    By Stan100 in forum C++ Programming
    Replies: 13
    Last Post: 06-22-2005, 04:06 PM

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