2

I'm writing this function to return a char pointer of reversed string.

void * PreverseStr (char str[]) 
{
    int size = strlen (str);
    char *returnstr = (char *)malloc (size * sizeof(char));
    for (int i = size - 1; i >= 0 ; i--)
    { 
        *returnstr = str[i];
        returnstr ++;
    }
    returnstr = 0;
    returnstr -= size;

    return returnstr ;
}

To test this function I wrote a main function like this

int main()
{   
    char str[] = "abcdefghijklmnopqrstuvwxyz";

    char *newstr = PreverseStr(str);
    printf("Reversed string using pointer: %s\n", newstr);

    free(newstr);

    return 0;
}

But it crashes before it could print out anything. I wonder what's wrong with my code. It would be much helpful if you can explain a fix to this.

Vlad from Moscow
  • 301,070
  • 26
  • 186
  • 335
  • 1
    `returnstr = 0;` is invalid. You've basically set your pointer to null. – Robert Harvey Mar 16 '22 at 17:10
  • 1
    Try simply saving the original pointer, and then restoring it after you run the loop. See https://replit.com/@robertwharvey/DesertedWorrisomeControlflowgraph#main.c. Or, simply return the saved pointer. – Robert Harvey Mar 16 '22 at 17:11
  • 1
    I'm pretty sure you meant to write `*returnstr = 0`. Nevertheless, 10 seconds in a debugger should make it extremely obvious what is happening. – Useless Mar 16 '22 at 17:17
  • @Useless the compiler does not return any warning and I don't know how to use a debugger yet. But the problem has been solved. Thnks all – Hibiki Supersanta Mar 16 '22 at 18:49
  • 1
    The compiler isn't obliged to warn you about this, and it's never too soon to learn how to use a debugger! – Useless Mar 16 '22 at 18:51

1 Answers1

2

For starters the return type void * makes no sense. The return type should be char *. As the function creates a new string without changing the source string then the function parameter should have the qualifier const.

This memory allocation

char *returnstr = (char *)malloc (size * sizeof(char));

allocates not enough space tp store the terminating zero character '\0' of the source string.

You need to write at least

char *returnstr = (char *)malloc ( ( size + 1 ) * sizeof(char));

After the for loop the pointer returnstr points to beyond the allocated memory because it is increased within the loop

returnstr ++;

Moreover after this assignment

returnstr = 0;

it becomes a null pointer.

The function can be declared and defined the following way

char * reverse_copy( const char s[] ) 
{
    size_t n = strlen( s );

    char *p = malloc( n + 1 );

    if ( p != NULL )
    {
        p += n;
        *p = '\0';

        while ( n-- )
        {
            *--p = *s++;
        }
    }

    return p;
}

Here is a demonstration program.

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

char *reverse_copy( const char s[] )
{
    size_t n = strlen( s );

    char *p = malloc( n + 1 );

    if (p != NULL)
    {
        p += n;
        *p = '\0';

        while (n--)
        {
            *--p = *s++;
        }
    }

    return p;
}

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

    puts( s );

    char *p = reverse_copy( s );

    if (p) puts( p );

    free( p );
}

Its output is

Hello, World!
!dlroW ,olleH
Vlad from Moscow
  • 301,070
  • 26
  • 186
  • 335