0

I tried to develop a function which take a string reverse letters and return pointer to string.

char *reverseStr(char s[])
{
    printf("Initial string is: %s\n", s);
    int cCounter = 0;
    char *result = malloc(20);

    while(*s != '\0')
    {
        cCounter++;
        s++;
    }
    printf("String contains %d symbols\n", cCounter);

    int begin = cCounter;

    for(; cCounter >= 0; cCounter--)
    {
        result[begin - cCounter] = *s;
        s--;
    }
    result[13] = '\0';
    return result;
}

in main function I invoke the function and tried to print the result in this way:

int main()
{
    char testStr[] = "Hello world!";
    char *pTestStr;

    puts("----------------------------------");
    puts("Input a string:");
    pTestStr = reverseStr(testStr);
    printf("%s\n", pTestStr);
    free(pTestStr);
    return 0;
}

but the result is unexpected, there is no reverse string. What is my fault?

Mike
  • 4,041
  • 6
  • 20
  • 37
Jura
  • 13
  • 1
  • 2
  • 5
    This is a great opportunity for you to [learn how to debug your programs](https://ericlippert.com/2014/03/05/how-to-debug-small-programs/). For example, by using a debugger you could step through your code, line by line, while watching variables and their values and how they change in real-time. – Some programmer dude Sep 25 '18 at 07:13
  • 4
    A hint though: To what character do `s` point when you start the second loop? What will the first character in `result` be because of that? – Some programmer dude Sep 25 '18 at 07:14
  • 3
    And this is very fishy: `result[13] = '\0';`. Hint: what is 13? And so is this: `char *result = malloc(20);`: why 20? – Jabberwocky Sep 25 '18 at 07:16
  • 5
    I failed to understand what is the use of two magic numbers here 20 and 13. Why not malloc() 1 + length of s – Rizwan Sep 25 '18 at 07:23
  • 2
    BTW: You might be interested in the [strlen](http://www.cplusplus.com/reference/cstring/strlen/?kw=strlen) function. – Jabberwocky Sep 25 '18 at 07:24
  • "The result is unexpected" does not tell us anything about your problem. Please be more precise in your problem statement. – Kami Kaze Sep 25 '18 at 07:52
  • Is it mandatory to allocate the string dynamically, or can you just reverse the input argument in-place? If the latter, this gets *much* simpler. And frankly, even dynamic, it shouldn't be this complex. – WhozCraig Sep 25 '18 at 08:03

2 Answers2

1

There are multiple mistakes in the shared code, primarily -

  • s++; move the pointer till '\0'. It should be brought back 1 unit to point to actual string by putting s--. Other wise the copied one will start with '\0' that will make it empty string.
  • Magic numbers 20 and 13. where in malloc() 1 + length of s should be sufficient instead or 20. For 13 just move a unit ahead and put '\0'

However, using string.h library functions() this can be super easy. But I think you are doing it for learning purpose.

Therefore, Corrected code without using string.h lib function() should look like this:

char *reverseStr(char s[])
{
    printf("Initial string is: %s\n", s);

    int cCounter = 0;
    while(*s != '\0')
    {
        cCounter++;
        s++;
    }
    s--; //move pointer back to point actual string's last charecter

    printf("String contains %d symbols\n", cCounter);

    char *result = (char *) malloc(sizeof(char) * ( cCounter + 1 ));
    if( result == NULL ) /*Check for failure. */
    {
        puts( "Can't allocate memory!" );
        exit( 0 );
    }

    char *tempResult = result;
    for (int begin = 0; begin < cCounter; begin++)
    {
        *tempResult = *s;
        s--; tempResult++;
    }
    *tempResult =  '\0';
    //result[cCounter+1] = '\0';
    return result;
}

Calling from main

int main()
{
    char testStr[] = "Hello world!";
    char *pTestStr;

    puts("----------------------------------");
    puts("Input a string:");
    pTestStr = reverseStr(testStr);
    printf("%s\n", pTestStr);
    free(pTestStr);
}

Output

----------------------------------
Input a string:
Initial string is: Hello world!
String contains 12 symbols
!dlrow olleH

As per WhozCraig suggestion just by using pointer arithmetic only -

char *reverseStr(const char s[])
{
    const char *end = s;
    while (*end)
        ++end;

    char *result = malloc((end - s) + 1), *beg = result;
    if (result == NULL)
    {
        perror("Failed to allocate string buffer");
        exit(EXIT_FAILURE);
    }

    while (end != s)
        *beg++ = *--end;
    *beg = 0;

    return result;
}
WhozCraig
  • 65,258
  • 11
  • 75
  • 141
Rizwan
  • 3,324
  • 3
  • 17
  • 38
  • Or just lose the counters and use pointer arithmetic. [like this](https://pastebin.com/ga58mSJB) – WhozCraig Sep 25 '18 at 08:22
  • @WhozCraig - Exactly, this will be even better in a precise way. Will update the answer. – Rizwan Sep 25 '18 at 08:25
  • why do i need to use an additional pointer to char (*tempResult)? While debugging (step-by-step) the tempResult contains only 1 char and result add each char. Why? at the end of the function terminal char '\0' makes tempResult empty, why? – Jura Sep 25 '18 at 12:38
  • @Jura - There is another implementation just below that, which don't use any tempResut. To explain this - tempResult is a forward moving pointer 1. if we don't keep a copy then we will loose starting pointer of the copied string. 2. it will only show the current copied character as it moved ahead a memory location after coping the previous character, while result is still pointing to the start of the memory location (start of the string) 3. tempResult - becomes empty because '\0' is the specification for null terminated string. And at the end tempResult only holds '\0' – Rizwan Sep 25 '18 at 13:22
  • now i got the idea, – Jura Sep 25 '18 at 13:24
  • @Jura - Please mark answer as correct, if it solves your problem. – Rizwan Sep 25 '18 at 13:26
0

Your code can be simplified using a string library function found in string.h

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

char *reverseStr(char s[])
{
    printf("Initial string is: %s\n", s);
    int cCounter = strlen(s);
    char *result = malloc(cCounter + 1);

    printf("String contains %d symbols\n", cCounter);

    int begin = cCounter;

    for(; cCounter > 0; cCounter--)
    {
        result[begin - cCounter] = s[cCounter - 1];
    }
    result[begin] = '\0';   
    return result;
}

int main()
{
    char testStr[] = "Hello world!";
    char *pTestStr;

    puts("----------------------------------");
    puts("Input a string:");
    pTestStr = reverseStr(testStr);
    printf("%s\n", pTestStr);
    free(pTestStr);
    return 0;
}

Output:

----------------------------------
Input a string:
Initial string is: Hello world!
String contains 12 symbols
!dlrow olleH
P.W
  • 26,289
  • 6
  • 39
  • 76