2

Trying to do Exercise 1-19 of K&R 2nd ed., e.g. writing a function to reverse a string. I thought I managed, but the print output looks strange :-) If I use STRINGSIZE 5 the output is Original String: hello Reversed String: ollehhello. If I use STRINGSIZE 6 to keep in mind the '\0' string end character and modify the while-loop to while ((outputString[STRINGSIZE - (i + 2)] = inputString[i]) != '\0'), then I get Original String: hello Reversed String: olleh?hello, I guess the ? is some random character coming from a '\0' added to the reversed string in the while-loop at position 5; but hello is again added. Could anyone explain how come hello gets added to the end of olleh and how can I get rid of it so that I only get the proper reversed string ?

Here is the code:

#include <stdio.h>
#define STRINGSIZE 5

void reverseString (char inputString[], char outputString[]);

int main(void) {
    char stringToReverse[] = "hello";
    char reversedString[STRINGSIZE]; 
    reverseString(stringToReverse, reversedString);
    printf("Original String: %s\nReversed String: %s\n", stringToReverse, reversedString);
}

void reverseString (char inputString[], char outputString[]) {
    int i;
    i = 0;
    while ((outputString[STRINGSIZE - (i + 1)] = inputString[i]) != '\0')
        ++i;
}
Mario Christov
  • 141
  • 1
  • 8
  • 1
    You didn't null-terminate the reversed string. Using `printf` on a character array which isn't null-terminated leads to undefined behavior. – John Coleman Jul 23 '17 at 13:25
  • 1
    There isn't even room in `reversedString[]` to hold `"hello"`, forwards or reversed. – ad absurdum Jul 23 '17 at 13:30
  • @JohnColeman Thank you, John, I think I implemented the 0-termination with `if (i == (STRINGSIZE)) { outputString[STRINGSIZE] = '\0'; break; } ` after the ++i in the while-loop, but the issue was obviously the declaration of the target string size as pointed out by @4386427 – Mario Christov Jul 23 '17 at 13:41

2 Answers2

2

First, the character array reversedString[] does not have enough space to store the null terminator of the string "hello". One option is to use a variable length array here:

char reversedString[strlen(stringToReverse) + 1];

VLAs were introduced in C99, and made optional in C11. As I remember it, K&R does not include coverage of variable length arrays, since even the 2nd edition was published before this.

Another option that would be compatible with C89 is to use the sizeof operator:

char stringToReverse[] = "hello";
char reversedString[sizeof stringToReverse];

Here, the result from the sizeof operator is known at compile time, and can be used in the declaration of a fixed size array. This size includes space for the null terminator, in contrast to the result from strlen("hello"). Note that this would not work with char *stringToReverse = "hello";, since then the sizeof operator would give the size of a pointer. This also would not work if stringToReverse had been passed into a function first, since then the array name would have decayed to a pointer to the first element of stringToReverse.

In the reverseString() function, the length of inputString needs to be determined (since STRINGSIZE is no longer being used); this can be done with strlen(), or in a loop. Then, critically, the function must be certain to add a null terminator (\0) to outputString[] before returning. Also note that a return statement has been added to the end of main() to make this truly C89 compatible:

#include <stdio.h>

void reverseString (char inputString[], char outputString[]);

int main(void) {
    char stringToReverse[] = "hello";
    char reversedString[sizeof stringToReverse];

    reverseString(stringToReverse, reversedString);
    printf("Original String: %s\nReversed String: %s\n",
           stringToReverse, reversedString);

    return 0;
}

void reverseString(char inputString[], char outputString[])
{
    int length = 0;
    int i = 0;

    /* Get inputString length; or use strlen() */
    while (inputString[length] != '\0') {
        ++length;
    }

    /* Copy to outputString[] in reverse */
    while (i < length) {
        outputString[i] = inputString[(length - i) - 1];
        ++i;
    }

    /* Add null terminator */
    outputString[i] = '\0';
}
ad absurdum
  • 19,498
  • 5
  • 37
  • 60
1

First I'll suggest that you change this line:

    char reversedString[STRINGSIZE]; 

to

char reversedString[strlen(stringToReverse) + 1];  // + 1 to make room for the string termination

Then I would do something like:

void reverseString (char inputString[], char outputString[]) {
    int i;
    int len = strlen(inputString);
    for(i=0; i<len; ++i)
    {
        outputString[len-i-1] = inputString[i];
    }
    outputString[len] = '\0';  // Terminate the string
}
Support Ukraine
  • 42,271
  • 4
  • 38
  • 63
  • Thank you, 4386427, for the answer, the generalization of the function with `strlen(inputString)` instead of `STRINGSIZE ` was also helpful :-) – Mario Christov Jul 23 '17 at 13:50
  • 5
    Note that this solution depends upon variable length arrays, which weren't in the Standard when K&R was published (hence, not covered in K&R). – ad absurdum Jul 23 '17 at 14:08
  • 2
    As noted, using `strlen()` to determine the size of `reversedString[]` would not be kosher for the language of K&R2 (it was published in 1989 or thereabouts, just about the time the original version of the standard was finalized; variable length arrays were added to C99, ten years later), there's an easy alternative that would work: `char reversedString[sizeof(stringToReverse)];`. Note that `sizeof` includes the terminal null too; no need to add 1. – Jonathan Leffler Jul 23 '17 at 14:52