unexpected results on simple string reverse algorithm

Question

I'm relatively new to C. I wanted to lern the language a bit by solving coderbyte challenges.

But I'm stucked at the first. It is supposed to be a simple String reverse algorithm.

When I input things like "asdf" or "1234567" the output is correct ("fdsa", "7654321"). But when I type "12345678" or "thisiscool" I get "87654321▒@"/"loocsisiht@" as a result. I don't know where the @ are comming from.

This is my code:

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

void FirstReverse(char str[]) {

  int len = strlen(str);
  char nstr[len];
  int i;

  for(i = 0; i < len; i++) {
      nstr[i] = *(str+len-1-i);
  }

  printf("%s\n", nstr);

}
int main(void) {

  char str[100];

  FirstReverse(gets(str));
  return 0;

}

Can someone please tell me where I can find the error? Thanks in advance :)

strlen returns the length without the nul termination, so nstr is too small. You also don't assign the nul termination. — user694733
– user694733, Commented Jun 28, 2016 at 10:45

user31264 · Accepted Answer · 2016-06-28 10:46:09Z

1

In C, strings are zero-terminated. A string "cat", for example, has 4 characters, and is represented as ('c','a','t',(char)0). You forgot about the final 0.

Note that strlen returns the string length without the final 0, so a string foo contains strlen(foo)+1 characters. Remember this when you allocate strings.

answered Jun 28, 2016 at 10:46

user31264

6,7824 gold badges30 silver badges42 bronze badges

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

Comments

Mark1987 · Accepted Answer · 2016-06-28 10:58:26Z

As the other answers have mentioned, you're missing a terminator. It should also be noted that it's bad practice to allocate strings the way you did. An array should always have a fixed size if you create it that way.

You should instead do:

char * nstr = malloc(sizeof(char) * (len+1));

Thereby allocating the size of each character (1 byte) times the lenght. Note the +1 because you need room for the string terminator.

When you call printf(, string); , it's gonna start from the first letter and print everything up to the terminator. Since you have no terminator here, it prints random characters, such as @.

What you're gonna wanna do to fix that, is adding:

nstr[i] = '\0';

after your loop.

Also remember to free the allocated memory.

marcolz · Accepted Answer · 2016-06-28 10:51:12Z

0

You forgot to allocate a char for the terminating '\0' in nstr[].

So, better use: char nstr[len + 1]; and set nstr[len] = 0;

Furthermore: gets() is evil: from the glibc manual page:

Never use gets(). Because it is impossible to tell without knowing the data in advance how many characters gets() will read, and because gets() will continue to store characters past the end of the buffer, it is extremely dangerous to use. It has been used to break computer security. Use fgets() instead.

edited Jun 28, 2016 at 10:51

answered Jun 28, 2016 at 10:44

marcolz

2,9902 gold badges27 silver badges30 bronze badges

Collectives™ on Stack Overflow

unexpected results on simple string reverse algorithm

3 Answers 3

Comments

Comments

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

3 Answers 3

Comments

Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Related