Right now, your code leaks memory--every time you call
reverse, it allocates some memory, and none of your other code frees it again.
If all you're doing is reversing one string, then exiting, that's of little consequence--but if you try to use this in real code, leaking memory like this is generally unacceptable.
Since you're not modifying the input string, you might as well use
const in the function's signature:
char* reverse(char const * str);
Buffer overrun protection
Right now you have:
This is essentially equivalent to
gets(word);. That is to say, it provides absolutely no protection against the user entering a string longer than you've provided space to store. When using
scanf (or cousins like
sscanf, etc.) you need to specify the maximum length:
Alternatively, consider using
fgets, which also requires you to specify the buffer size.
Note that there's a difference in the size you specify though. With
fgets, you specify the size of the buffer, but with
scanf you specify the number of characters it's allowed to read, which is one less than the size of the buffer itself. Also note that
fgets normally retains the
\n at the end of what was entered though (if you get data without a
\n on the end, it means the buffer you supplied wasn't large enough to hold all the data that was entered).
When you allocate your memory:
char* reversed = (char*) malloc(sizeof(char) * strlen(str) + 1);
...you're currently casting the result of
malloc. This is generally frowned upon by C programmers--it's unnecessary and it can cover up a bug of having failed to include the right header to declare/prototype
char* reversed = malloc(sizeof(char) * strlen(str) + 1);
The cast is only necessary if you decide to write C that can also compile as C++. This tends to give the worst of both worlds, and should generally be avoided--if you're going to compile this as C++, use
When you allocate memory, you multiply the length by
sizeof(char) is guaranteed to be 1, so the multiplication accomplishes nothing.
Even if you decide to leave the multiplication in (since it is necessary for types other than
char), I prefer to use code like this:
char* reversed = malloc(sizeof(*reversed) * strlen(str) + 1);
This has the advantage that when/if you (for example) decide to support wide characters, you can change it to something like:
wchar_t* reversed = malloc(sizeof(*reversed) * (wcslen(str) + 1));
...so the argument to
sizeof doesn't need to change at all (and believe me--if you decide to do something like this, you have enough headaches to deal with, so even a small help will be welcome).