0

I'm kind of in a situation where I need to do some C-String concatenation, so I decided to take it upon my self to learn ( this is just a personal project so I have all the time in the world, really ).

So far I've come up with these two functions (explanation given below):

static inline  // make inline to leave out the stack manipulation
size_t StrLength( const char* string )
{
    return strnlen( string, strlen( string ) );
}

static void concatstr( char** dest, const char** src )
{
    const size_t destlen = StrLength( *dest );
    const size_t srclen  = StrLength( *src );

    const size_t total_len    = destlen + srclen; 

    const size_t totalLenNull = total_len + 1;

    char* tmp = ( char* )malloc( sizeof( char ) * totalLenNull ); //<-- Because of this...

    size_t counter = 0;

    for( size_t iDest = 0; iDest < destlen; ++iDest )
        tmp[ counter++ ] = *dest[ iDest ];

    for ( size_t iSrc = 0; iSrc < srclen; ++iSrc ) 
        tmp[ counter++ ] = *src[ iSrc ];

    *dest = ( char* ) realloc( *dest, totalLenNull ); 

    strncpy( *dest, tmp, total_len ); 

    free( tmp );

    tmp = NULL;
}

The idea behind the inline StrLength function is that, if I understand inline correctly, it's supposed to work like a safer macro in C++, so it's less bug prone (I think). I made it inline since it's just one line of code, and a stack manipulation for that kind of a process seems a bit much. If I'm wrong, however, please correct me on this.

Now, onto the concatstr() function:

The problem here is that as soon as I get to the second loop, the program crashes upon the 2nd iteration due to a memory read violation. Why this is happening I'm not sure.

The calling code looks as follows:

const size_t len = StrLength( msg ) + mPrefix.length() + StrLength( "\n\n" );

char* out = ( char* )malloc( sizeof( char ) * ( len + 1 ) );

out[0] = '\0';

const char* tmp_pass = mPrefix.c_str();

concatstr( &out, &tmp_pass );
concatstr( &out, &msg );

The only reason I'm putting a visual-C++ tag here is that even though I'm doing this like one would in straight c, I'm using VC++ as a compiler.

Does anyone have an idea of what the issue is here?

zeboidlund
  • 9,731
  • 31
  • 118
  • 180
  • 1
    You don't need `StrLength()`; especially after reading the code of that function; use `strlen()` directly. Please see http://en.cppreference.com/w/cpp/language/operator_precedence. `[]` takes precedence over `*` and you use that in 2 places. – foxx1337 Dec 01 '12 at 23:22

1 Answers1

2

You have the precedences wrong,

tmp[ counter++ ] = *dest[ iDest ];

is (implicitly) parethesised

tmp[ counter++ ] = *(dest[ iDest ]);

but you need

tmp[ counter++ ] = (*dest)[ iDest ];

with explicit parentheses.

Without the parentheses, the (presumed) char* at an offset of iDest * sizeof(char*) bytes after *dest is dereferenced, there probably isn't a valid char* that you are allowed to dereference. You want to access the byte at an offset of iDest bytes after what *dest points to.

At the end,

strncpy( *dest, tmp, total_len );

doesn't 0-terminate the concatenated strings. You need totalLenNull there.

Aside:

return strnlen( string, strlen( string ) );

is rather circumspect. First you traverse string to find the terminating 0-byte, counting chars before that. Then you check whether there is a terminating 0-byte among the first strlen(string) bytes of string. Unless string is modified between these two calls (and then you're probably hosed), the second will return exactly the same as the first call.

Daniel Fischer
  • 181,706
  • 17
  • 308
  • 431