Bad memory access while copying string without space

Question

This is the structure that i have

typedef struct {
    int  startIndex;
    int length;
    char *rawString;
}Tokenizer;

I got a function to copy string (Which will trim out the space)

void copyStringWithoutSpace(char *source,char *destination )
{
    int i =0 ,j=0;

    destination = malloc (sizeof(char)*strlen(source));
    for(i=0;i<strlen(source);i++)
    {
        if(!(source[i]==' ')||(source[i]=='\t'))
        {
            destination[j] =source[i];
            j++;
        }
    }
    destination[j]='\0';
}

And this is the function that call the copyStringWithoutSpace

Tokenizer *initTokenizer(char *expression)
{
    int i =0, j=0;
    Tokenizer *newTokenizer = malloc (sizeof(Tokenizer));
    copyStringWithoutSpace(expression, newTokenizer->rawString);
    newTokenizer ->startIndex =0;
    newTokenizer ->length =strlen(newTokenizer->rawString);
    return newTokenizer;
}

Now, this code will return a bad memory access. I have troubleshooting for so long and cant solve. Anyone wish to help me ?

Answer 1

Be careful that C strings are NULL character terminated, and this character is not taken into account by strlen, meaning that its size in memory is actually strlen(source) + 1.

What you need to do is to allocate your buffer like this:

destination = malloc (strlen(source) + 1);

sizeof(char) is guaranteed to be 1 by the C standard, so you can omit it safely.

Also, you're modifying the value of the destination variable inside the copyStringWithoutSpace function, which means the newly allocated memory will not be visible from outside the function, and this will result in a memory leak.

You need to either return the pointer, and have the following signature:

char * copyStringWithoutSpace(char *source)

or alternatively:

void copyStringWithoutSpace(char *source, char ** destination)

where you'll have to allocate the memory like this:

*destination = malloc (strlen(source) + 1);

Another error here is this: if(!(source[i]==' ')||(source[i]=='\t'))

This does not do what you want, because of operator precedence. Here, the negation operator is only applied to the following pair of parenthesis, meaning your test can be spelled as this:

if source[i] is not a space, or source[i] is a tabulation

You should have written it like this:

if (source[i] != ' ' && source[i] != '\t')

Which is much clearer, isn't it ?

Then also, as pointed out in a comment, calling strlen at each iteration is very inefficient, because the whole string needs to be iterated until the NULL character.