C: reverse string function not affecting pointer

Question

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

int reverse(char *, int);

main()
{
    char *word = "Thanks for your help";

    reverse(word, strlen(word));
    printf("%s", word);     

    getchar();

}

int reverse(char *line, int len)
{
    int i, j;
    char *newline = malloc(strlen(line));

    for (i = len - 1, j = 0 ; i >= 0; i--, j++)
    {
        newline[j] = line[i];
    }
    newline[j] = '\0';
    line = &newline;
}

Hey folks. I've got a simple C question that I can't seem to solve.

The program above is meant to take in a string and print it out backwards. Reverse is the function by which this is done.

The issue, specifically, is that when I print word in main(), the string appears unchanged. I've attempted to make the address of line the address of newline, but it doesn't have any effect.

Arguments are passed by value; changing what the line in reverse points to doesn’t do the same in its caller. If you want to produce a new string, you should probably just return a char*; if the intent is to reverse the string in place, you can do that (and have to, by definition) without allocating extra space. — Ry-
– Ry- ♦, Commented Nov 15, 2015 at 23:18
line = &newline; is illegal - pay attention to compiler messages — M.M
– M.M, Commented Nov 15, 2015 at 23:24

Jonathon Reinhart · Accepted Answer · 2015-11-15 23:32:39Z

int reverse(char *line, int len)
{
    int i, j;
    char *newline = malloc(strlen(line));

    for (i = len - 1, j = 0 ; i >= 0; i--, j++)
    {
        newline[j] = line[i];
    }
    newline[j] = '\0';
    line = &newline;             // Your problem is here
}

You're merely assigning to the local line pointer. This has no effect on the calling function whatsoever.

Consider instead:

char *reverse(char *line, int len)
{
    // ...
    return newline;
}

Additional advice:

Turn on compiler warnings, and heed them. You've got lots of little things wrong (e.g. reverse isn't currently returning anything, but is declared as returning int).
Given that the first argument of reverse is a pointer to a C string (NUL-terminated), there's no need to take a length argument as well.
A reverse function doesn't necessarily need to be defined as returning a copy of the string, reversed. It could instead reverse a string in-place. Note that you cannot pass a string literal to a function like this, as they are read-only.

Here's how I would write this:

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

void reverse(char *str)
{
    size_t i, j;
    for (i = strlen(str) - 1, j = 0 ; i > j; i--, j++)
    {
        // Swap characters
        char c = str[i];
        str[i] = str[j];
        str[j] = c;
    }
}

int main(void)
{
    // Mutable string allocated on the stack;
    // we cannot just pass a string literal to reverse().
    char str[] = "Here is a test string";

    reverse(str);

    printf("Result: \"%s\"\n", str);
    return 0;
}

Note that the for loop condition is i > j, because we want each to only traverse half the array, and not swap each character twice.

Result:

$ ./a.exe
Result: "gnirts tset a si ereH"

Alex Lew · Accepted Answer · 2015-11-15 23:26:41Z

Take a look at the code below:

void addOne(int a) {
   int newA = a + 1;
   a = newA;
}

int main() {
  int num = 5;
  addOne(num);
  printf("%d\n", num);
}

Do you see why that will print 5, and not 6? It's because when you pass num to addOne, you actually make a copy of num. When addOne changes a to newA, it is changing the copy (called a), not the original variable, num. C has pass-by-value semantics.

Your code suffers from the same problem (and a couple other things). When you call reverse, a copy of word is made (not a copy of the string, but a copy of the character pointer, which points to the string). When you change line to point to your new string, newLine, you are not actually changing the passed-in pointer; you are changing the copy of the pointer.

So, how should you implement reverse? It depends: there are a couple options.

reverse could return a newly allocated string containing the original string, reversed. In this case, your function signature would be char *reverse, instead of int reverse.
reverse could modify the original string in place. That is, you never allocate a new string, and simply move the characters of the original string around. This works, in general, but not in your case because char pointers initialized with string literals do not necessarily point to writable memory.
reverse could actually change the passed-in pointer to point at a new string (what you are trying to do in your current code). To do this, you'd have to write a function void reverse(char **pointerToString). Then you could assign *pointerToString = newLine;. But this is not great practice. The original passed-in argument is now inaccessible, and if it was malloc'd, it can't be freed.

Collectives™ on Stack Overflow

C: reverse string function not affecting pointer

2 Answers 2

Comments

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Related