Realloc gives error: _CrtIsValidHeapPointer(block)

Question

I have this school assignment where we are given some code to modify. The part im having trouble with is to truncate a dynamically allocated string. We are writing a function where we take in a pointer to the string and the new length of the string (as an int).

void dstring_truncate(DString* destination, unsigned int truncatedLength)
{

    assert(destination != NULL);
    assert(*destination != NULL);
    assert(truncatedLength > 0);

    DString temp = (DString)malloc(strlen(*destination) * sizeof(char));
    strcpy(temp, *destination);
    printf("\n%s test dstring truncate\n", temp);
    temp[truncatedLength] = '\0';
    temp = (DString)realloc(temp, truncatedLength + 1);

    *destination[truncatedLength] = '\0';
    *destination = (DString)realloc(*destination, truncatedLength + 1);
    printf("%s test dstring truncate after realloc\n", *destination);
    
}

The call to the function looks like thisdstring_truncate(&str1, 10); DString is the ADT we are working with typedef char* DString;

also the first function in the program initializes the string DString dstring_initialize(const char* str);

I assume its me not understanding pointers because the "Temp" in the code above works like intended but the "*destination" does not.

Ive tried using "destination", "*destination", "**destination" and i've rewatched the material given and some youtube/documentation online but i just cant get my head around this.

also this is how i wrote the initialize code maybe there is something wrong here?

DString dstring_initialize(const char* str)
{
    assert(str != NULL);
    DString arr = (DString)malloc((strlen(str) + 1) * sizeof(char));
    if (arr == NULL) {
        printf("Memory allocation Failed");
        return NULL;
    }
    strcpy(arr, str);
    if (strcmp(str, arr) == 0) 
    {
        return arr; // Ersatt denna rad. Den ar just nu med for att programmet ska kompilera
    }
}

Answer 1

Using the varable temp as is within the function does not make sense because at least it produces a memory leak. I think you included the code with the variable temp temporary only for the purpose of testing.

Nevertheless this part of the code contains a bug in this statement

 DString temp = (DString)malloc(strlen(*destination) * sizeof(char));

You did not reserve memory for the terminating zero character '\0' of the created string. Thus this statement

strcpy(temp, *destination);

overwrites memory beyond the allocated dynamically extent of memory that already results in undefined behavior.

This statement

temp[truncatedLength] = '\0';

is logically also incorrect because you are not checking that the value of truncatedLength is indeed not greater than the length of the source string. Before reallocting a string you need to check that the value of truncatedLength is less than the currect length of the source string.

By the way the variable (the function parameter) truncatedLength should have type size_t instead of unsigned int. size_t is the return type of the function strlen and the type of parameters that specify sizes in standard memory allocation functions.

In this statement

*destination[truncatedLength] = '\0';

there is used invalid order of operators. Instead you need to write

( *destination )[truncatedLength] = '\0';

And again you need at first to check that truncatedLength is not greater than the length of the source string.

The function realloc can return a null pointer. In this case you will have a memory leak because the value of the pointer *destination will be overwritten in this statement

*destination = (DString)realloc(*destination, truncatedLength + 1);

You need to use an intermediate variable to which the returned value of the function realloc will be assigned.

And you should report to the caller of the function whether realloction was successful. So it is better to declare the return type of the function as int instead of void.

Here is a demonstration program that shows how the functions can be updated

#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <assert.h>

typedef char *DString;

DString dstring_initialize( const char *str )
{
    assert( str != NULL );

    DString arr = malloc( strlen( str ) + 1 );

    if ( arr == NULL)
    {
        puts( "Memory allocation Failed" );
    }
    else
    {
        strcpy( arr, str );
    }

    return arr;
}

int dstring_truncate( DString *destination, size_t truncatedLength )
{

    assert( destination != NULL );
    assert( *destination != NULL );
    assert( truncatedLength > 0 );

    size_t n = strlen( *destination );

    int success = truncatedLength < n;

    if ( success )
    {
        DString temp = realloc( *destination, truncatedLength + 1 );

        success = temp != NULL;

        if (success)
        {
            *destination = temp;
            ( *destination )[truncatedLength] = '\0';
        }
    }

    return success;
}

int main( void )
{
    const char *s = "Hello Wordl!";

    DString ds = dstring_initialize( s );

    if ( ds != NULL )
    {
        int success = dstring_truncate( &ds, strchr( ds, ' ' ) - ds );

        if (success)
        {
            printf( "Now after truncation the string is \"%s\"\n", ds );
        }

        free( ds );
    }
}

The program output is

Now after truncation the string is "Hello"

Actually the function dstring_initialize should not issue any message. It is the caller of the function based on the return value of the called function will decide whether to issue a message or not. So the function should look the followng wat

DString dstring_initialize( const char *str )
{
    assert( str != NULL );

    DString arr = malloc( strlen( str ) + 1 );

    if ( arr != NULL)
    {
        strcpy( arr, str );
    }

    return arr;
}