I was messing around with an exercise I found.
This seems to work, except in debugging, some VERY odd stuff happens,
so I thought I'd chuck the code on here to see how it stands up.
Very odd stuff indeed.
I had to make char *y global in order to free it. Disgusting.
No, you didn't. See the lines I added to your main().
I'm going to pretend you used a much smaller original string for
illustrative purposes, namely "123".
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
char *rev_str(char *);
Make the prototype:
char *rev_str(const char *);
....since you don't modify the input string and the function is safe
for string literals and constant strings.
char *y;
int main(void)
{
char *stToRev = "Dontchajustwishthisstringwouldgetreversed";
char *rev = rev_str(stTdRev);
printf("%s\n", rev_str(stToRev));
free(y);
Now replace the two lines above with:
printf("%s\n", rev);
free(rev);
return 0;
}
char *rev_str(char *x)
Again:
char *rev_str(const char *x)
{
size_t arrLength = strlen(x)+1;
Good, adding the +1. For the string "123", arrLength is 4.
Bad, not checking for a NULL return from malloc().
memcpy(y,x,arrLength);
y = &y[arrLength-1];
Both lines above invokes undefined behavior if malloc() was
unsuccessful and returned a null pointer. Also it is completely
unnecessary, what do you gain by copying the contents in the original
order if you are only going to write over them with the reverse order?
If all you wanted to do was '\0' terminate the new string, it would
surely be more efficient, and much clearer, to do:
y += arrLength - 1;
*y = '\0';
For the moment, let's assume your memcpy() code is still in place, so
as this loop starts, here is what the two memory areas and the two
pointers look like on each pass through the loop:
Before pass 1: orig = 1 2 3 \0 rev = 1 2 3 \0
x y
Before pass 2: orig = 1 2 3 \0 rev = 1 2 1 \0
x y
Before pass 3: orig = 1 2 3 \0 rev = 1 2 1 \0
x y
Before pass 3: orig = 1 2 3 \0 rev = 3 2 1 \0
x y
Before pass 4: orig = 1 2 3 \0 rev = 3 2 1 \0
x y
During pass 4, you first decrement 'y' to point to one byte before the
allocated memory, which is in itself undefined behavior. You can
always form a pointer to the object one past the end of valid memory,
whether it is dynamically allocated or not, but there is no guarantee
that you can form a pointer before the beginning of the block.
But even worse, you write, or attempt to write, a '\0' null character
from the initial string into this byte one before the allocated
memory. That's a serious error, and most likely the cause of your
"odd" behavior.
It's too late now, because your program is already off into
never-never land, but you don't need to dereference the pointer just
to increment it, and you don't need to compute the value before the
return statement.
You have no use for this, because when you rewrite the algorithm
properly you will never point to before the beginning of the allocated
block and need to increment the pointer. But if you did need to
increment a pointer before returning its value, you could combine
these two lines into one, either:
return ++y;
....or:
return y + 1;
