Newbie need help with strtok and calloc

[inlandpac]

Hello,

This is my first program in C, so please don't be too critical

I have a config file parser I wrote that is really causing some problems. I had it working and literally all-of-a-sudden it stopped working!

Here is the code:

File: program_definitions.cfg

SCRIPT_NAME=Currency Conversion
CONVERT_TO_LABEL=US Dollars
STRING_DIVIDER=----------------------------------------

File: config_parser.h

#include <iostream>
#include <string>
#include <ctype.h>

int config_parser(char *fileName);
char *TrimSpaces(char *str);

char *SCRIPT_NAME;
char *STRING_DIVIDER;
char *CONVERT_TO_LABEL;

int config_parser(char *fileName)
{
	FILE *filePtr;

	if( (filePtr = fopen( fileName, "r" )) == NULL )
	{
		printf("%s -- Unable to open configuration file.%s","Error!",fileName);
		exit( 1 );
	}
	else
	{
		while( !feof(filePtr) )
		{
			char data[255];
			char *pt;
			char *tmp_tok_ptr;

			fgets(data, 255, filePtr);

			//printf("data = %s\n",data);

			if( (data[0] != '\n') && (data[0] != '#') )
			{
				//printf("data[0] = [%c]\n",data[0]);

				pt = strtok(data, "=");

				printf("pt = [%s]\n",pt);

				if(strcmp(pt,"SCRIPT_NAME") == 0)
				{
					tmp_tok_ptr = strtok(NULL, "=");
					SCRIPT_NAME = (char *) calloc(strlen(tmp_tok_ptr), sizeof(char));
					strncpy(SCRIPT_NAME,tmp_tok_ptr,strlen(tmp_tok_ptr));
					SCRIPT_NAME = TrimSpaces(SCRIPT_NAME);

					//printf("SCRIPT_NAME = [%s]\n",SCRIPT_NAME);
				}
				else if(strcmp(pt,"STRING_DIVIDER") == 0)
				{
					tmp_tok_ptr = strtok(NULL, "=");
					STRING_DIVIDER = (char *) calloc(strlen(tmp_tok_ptr), sizeof(char));
					strncpy(STRING_DIVIDER,tmp_tok_ptr,strlen(tmp_tok_ptr));
					STRING_DIVIDER = TrimSpaces(STRING_DIVIDER);

					//printf("STRING_DIVIDER = [%s]\n",STRING_DIVIDER);
				}
				else if(strcmp(pt,"CONVERT_TO_LABEL") == 0)
				{
					tmp_tok_ptr = strtok(NULL, "=");
					CONVERT_TO_LABEL = (char *) calloc(strlen(tmp_tok_ptr), sizeof(char));
					strncpy(CONVERT_TO_LABEL,tmp_tok_ptr,strlen(tmp_tok_ptr));
					CONVERT_TO_LABEL = TrimSpaces(CONVERT_TO_LABEL);

					//printf("CONVERT_TO_LABEL = [%s]\n",CONVERT_TO_LABEL);
				}
			}
		}

		fclose(filePtr);
	}

	return 0;
}

/**
 * Removes left and right spaces from a string
 *
 * @param str  String to trim
 *
 * @return  char* to the trimed string
 */
char *TrimSpaces(char *str)
{
	size_t len;
	char *right, *left;

	/* Trim whitespace from left side */
	for (left = str; isspace(*left); left++);

	/* Trim whitespace from right side */
	if ((len = strlen(left)))
	{
		right = left + (len - 1);

		while (isspace(*right))
		{
			*right = '\0';
			right--;
		}
	}

	/* Only do the str copy if their was spaces to the left */
	if (left != str)
		strcpy(str, left);

	return (str);
}

And I have a program that loads config_parser.h and calls the config_parser() method and passes in the name of the config file.

When I compile and then attempt to execute the program, I get a core dump and the following from the debugger:

>	currency_conversion.exe!config_parser(char * fileName=0x0040a298)  Line 53 + 0x8	C++
 	currency_conversion.exe!main(int argc=1145656905, char * * argv=0x2d005245)  Line 77	C++
 	currency_conversion.exe!mainCRTStartup()  Line 259 + 0x12	C
 	kernel32.dll!77e814c7()

Line 53 is:

STRING_DIVIDER = (char *) calloc(strlen(tmp_tok_ptr), sizeof(char));

Any help would be appreciated!

ICQ: 54380631

 

[ArkM]

1. There is C Forum here and your presents a program in C, but why it has C++ headers included? You must include <stdio.h> (not <iostream>) and <string.h> (not <string>) C headers.
2. No need to write 1st else clause: your then clause has exit(1). Redundant indentation...
3. It seems no need in strtok() in your case. Try this (in line input loop):

char* pval = strchr(data,'=');
if (!pval)
   continue; /* it's not a keyword definition line */
*pval++ = 0; /* Now pval refers to the start of value */
/* and start of data == start of keyword string */

That's all with your 'lexical analyser'.
4. You forget to allocate a space for a string terminator zero byte: true slot for a dynamic C-string copy is strlen(str)+1. Now your program memory crashed. Then strncpy() copies only n-1 byte (and don't add zero terminator if its source string longer than strncpy 2nd arg value. Try:

char* StrDup(const char* str)
{
  char* p = 0;
  int   sz;
  if (p)
  {
     sz = strlen(str) + 1;
     p = malloc(sz);
     memcpy(p,str,sz);
  }
  return p;
}

5. It's an error to use strcpy with overlapped strings (see strcpy specs). Your TrimSpaces is incorrect. Use memmove() instead of strcpy.
6. Don't declare local (stack) vars in a loop body. Your program allocates them in every loop (it's redundant activity)...
7. Don't use all uppercases for names of variables. Traditionally these names reserved for macros (constants) only.

Redesign your code and try again...
Good luck!

 

[Salem]

> 6. Don't declare local (stack) vars in a loop body. Your program allocates them in every loop (it's redundant activity)...
I've never seen a C compiler do this. It always allocates all local variables at the start of the function, and then just uses the language scope rules to enforce access to those variables.

> while( !feof(filePtr) )
The classic 'it processes the last line twice' problem here
Use

while ( fgets(data, 255, filePtr) != NULL ) {
  /* your code here */
}

--

 

[ArkM]

Alas, let's see on C semantics...
Loop body block (i.e. { declaraions...} ) must allocate its local variables every time... Common trap in block contained languages. The C has true blocks, not only scope areas...
Please, try this:

int i;
for (i = 0; i < 3; ++i)
{
  int j = 0;
  ++j;
  printf("%d\t%d\n",i,j);
}
/* Compare with: */
int j = 0;
for (i = ....)
...

 

[Salem]

You're confusing allocation (when the compiler allocates space for a variable), with initialisation (when a value is written to a variable).
Your example obviously initialises j each time around the loop, but it does not mean the space set aside for it is created and destroyed each time.

int func1 (int x, int y)
{
  int i;
  for ( i = 0 ; i < 10 ; i++ ) {
    int j = i*2;
    printf( "%d\n", j );
  }
}

int func2 (int x, int y)
{
  int i,j;
  for ( i = 0 ; i < 10 ; i++ ) {
    j = i*2;
    printf( "%d\n", j );
  }
}

The generated prolog for these two functions is the same

_func1:
	pushl	%ebp
	movl	%esp, %ebp
	pushl	%ebx
	subl	$20, %esp
 
_func2:
	pushl	%ebp
	movl	%esp, %ebp
	pushl	%ebx
	subl	$20, %esp

The space is set aside at the start of the function.
The scope rules define the access to those variables. When something goes out of scope, it is hidden, not destroyed.


--

 

[inlandpac]

Thanks, all!

I will work through all of this (a lot of information here) and will see what I can do.

I appreciate your help!


ICQ: 54380631

 

[ArkM]

I'm not confusing about storage duration, memory allocation and object initialization. Yes, in C a compiler CAN calculate maximum stack size and allocate inner blocks variables onto the proper stack frame in compile time. But it is not obliged to do that. Any simple-minded compiler may generate block prologue in a loop body - it's not an error, it's semantically consistent approach.

In other C-like languages (more precisely, languages with block statements) the real block prologue code may be included in a loop body (e.g. ctors call if inner block has class local variables).

In that case the buffer array pragmatically belongs to the loop as whole. It's the other question when the compiler allocates memory and when initialize allocated memory: semantically (and logically) we have a variable with automatic duration - that's all.

Hope I explain my point of view...