Print a string reversed in C

Question

I'm coding a program that takes some files as parameters and prints all lines reversed. The problem is that I get unexpected results:

If I apply it to a file containing the following lines

one
two
three
four

I get the expected result, but if the file contains

september
november
december

It returns

rebmetpes
rebmevons
rebmeceds

And I don't understand why it adds a "s" at the end

Here is my code

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

void reverse(char *word);

int main(int argc, char *argv[], char*envp[]) {
    /* No arguments */
    if (argc == 1) {
        return (0);
    }

    FILE *fp;

    int i;
    for (i = 1; i < argc; i++) {
        fp = fopen(argv[i],"r"); // read mode

        if( fp == NULL )
        {
            fprintf(stderr, "Error, no file");
        }

        else
        {
            char line [2048];
/*read line and reverse it. the function reverse it prints it*/
            while ( fgets(line, sizeof line, fp) != NULL )
                reverse(line);  
        }
        fclose(fp);
    }

    return (0);
}

void reverse(char *word)
{
    char *aux;
    aux = word;
    /* Store the length of the word passed as parameter */
    int longitud;
    longitud = (int) strlen(aux);

    /* Allocate memory enough ??? */
    char *res = malloc( longitud * sizeof(char) );

    int i;

    /in this loop i copy the string reversed into a new one
    for (i = 0; i < longitud-1; i++)
    {
        res[i] = word[longitud - 2 - i];
    }

    fprintf(stdout, "%s\n", res);
    free(res);
}

(NOTE: some code has been deleted for clarity but it should compile)

when you malloc for string, measuring length using strlen(), you need +1 for NULL terminator. — moeCake
– moeCake, Commented Nov 22, 2013 at 11:10

haccks · Accepted Answer · 2013-11-22 11:17:51Z

4

You forget to terminate your string with \0 character. In reversing the string \0 becomes your first character of reversed string. First allocate memory for one more character than you allocated

char *res = malloc( longitud * sizeof(char) + 1);

And the try this

for (i = 0; i < longitud-1; i++)
{
    res[i] = word[longitud - 2 - i];
}
res[i] = '\0'; // Terminating string with '\0'

edited Nov 22, 2013 at 11:17

answered Nov 22, 2013 at 11:07

haccks

107k28 gold badges181 silver badges274 bronze badges

Sign up to request clarification or add additional context in comments.

2 Comments

M Oehm Over a year ago

Yes, but strlen gives you the length without the trailing zero, so you should malloc(longitud + 1) to have room for the '\0'. (The sizeof(char) is guaranteed to be 1.) Alternatively you can use the precision modifier in the format string to write out at most longitud chars like so: printf("%.*s\n", longitud - 1, res);

haccks Over a year ago

@MOehm; I missed char *res = malloc( longitud * sizeof(char) ); line. Adding to my answer.

slugonamission · Accepted Answer · 2013-11-22 11:10:49Z

I think I know the problem, and it's a bit of a weird issue.

Strings in C are zero terminated. This means that the string "Hi!" in memory is actually represented as 'H','i','!','\0'. The way strlen etc then know the length of the string is by counting the number of characters, starting from the first character, before the zero terminator. Similarly, when printing a string, fprintf will print all the characters until it hits the zero terminator.

The problem is, your reverse function never bothers to set the zero terminator at the end, which it needs to since you're copying characters into the buffer character by character. This means it runs off the end of your allocated res buffer, and into undefined memory, which just happened to be zero when you hit it (malloc makes no promises of the contents of the buffer you allocate, just that it's big enough). You should get different behaviour on Windows, since I believe that in debug mode, malloc initialises all buffers to 0xcccccccc.

So, what's happening is you copy september, reversed, into res. This works as you see, because it just so happens that there's a zero at the end.

You then free res, then malloc it again. Again, by chance (and because of some smartness in malloc) you get the same buffer back, which already contains "rebmetpes". You then put "november" in, reversed, which is slightly shorter, hence your buffer now contains "rebmevons".

So, the fix? Allocate another character too, this will hold your zero terminator (char *res = malloc( longitud * sizeof(char) + 1);). After you reverse the string, set the zero terminator at the end of the string (res[longitud] = '\0';).

Jekyll · Accepted Answer · 2013-11-22 11:19:33Z

1

there are two errors there, the first one is that you need one char more allocated (all chars for the string + 1 for the terminator)

    char *res = malloc( (longitud+1) * sizeof(char) );

The second one is that you have to terminate the string:

   res[longitud]='\0';

You can terminate the string before entering in the loop because you know already the size of the destination string.

Note that using calloc instead of malloc you will not need to terminate the string as the memory gets alreay zero-initialised

edited Nov 22, 2013 at 11:19

answered Nov 22, 2013 at 11:11

Jekyll

1,44412 silver badges15 bronze badges

Comments

Guimo · Accepted Answer · 2013-11-22 11:57:24Z

0

Thanks, it solved my problem. I read something about the "\0" in strings but wasn't very clear, which is now after reading all the answers (all are pretty good). Thank you all for the help.

answered Nov 22, 2013 at 11:57

Guimo

7521 gold badge7 silver badges18 bronze badges

Collectives™ on Stack Overflow

Print a string reversed in C

4 Answers 4

2 Comments

Comments

Comments

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

4 Answers 4

2 Comments

Comments

Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Related