C reverse string exercise

You already found a bug but please keep reading.

Comments in the code.

#include <stdio.h>


char *string_end(char *str);
void string_reverse(char *str);


int main()
{
    char string[30];
    printf("Give a string and I will reverse it: ");
    scanf("%s" , string);
    printf("Your string: %s\n\n" , string);
    printf("The reversed string: \n");
    string_reverse(string);
    return 0;
}

/* This function is not needed as it will be shown below */
char *string_end(char *str)
{
    while(1)
    {
        if(*str == '\0')
        {
            str =  --str;
/* Think what happens if str is an empty string: You would be returning a pointer
to the location BEFORE the start of string. This is not necessarily a bad thing,
but in the line below you are accessing that location. This IS a bad thing.*/
            //printf("Here->%c|%p\n",*str,str);
            break;
        }
        printf("%c|%p\n",*str,str);
        str++;
    }
    //printf("\n\n\n");
    return str;
}


void string_reverse(char *str)
{
    //printf("%c|%p\n\n",*str,str);

    char *start = str; //Not needed
/* Except for the last printf statement, you don't really need another variable.
In C functions are called by value. You can use the parameter any way you want.
It will not be returned to caller. */

    char temp;
//    char *end = string_end(str);

    char* end = str + strlen(str)-1; //do like this
/* strlen function returns the length of string not counting the final \0
The following line gives you a pointer to the last character in the string
IF THE STRING IS NOT EMPTY.*/

    //printf("%c|%p\n",*end,end);
    //printf("%c|%p\n",*start,start);

/* Think twice before using do...while loops. In most cases using a while...
loop is easier to read and safer. In this case, again, if the string is empty
you trash some memory.
    do
    {
        temp = *start;
        *start = *end;
        *end = temp;
    }
    while(start++ != end--);
*/
    while (start < end) //takes care of empty string
    {
       temp = *start;
       *start++ = *end;
       *end++ = temp;
    }
    printf("\n%s\n" , str);
}

You can find the problem quite easily with the debugger.

The while statement in your reverse code is making a bad assumption. It's assuming that under all circumstances start will eventually equal end. That's never the case if you have an even number of characters. For example if start and end are 5 and 6, on the next iteration, they will be 6 and 5.

There is some odd code in there ...

str =  --str;

That's nasty ... I really don't recommend that at all. The problem is that what you are using is a side effect operator, and exactly what it will give you is dependant on the compiler writer: Why does x = ++x + x++ give me the wrong answer?[^]
And using a loop that has no terminator is not a good idea.
I'd do it differnetly:
Create a second array of chars the same size as the input buffer plus one char. Put a null in the last element to terminate the string.
Use a loop that continues until it finds a null in the input string.
Place the first character of the input string just before the null you added to the output.
Move on to the next character.
Place the second character of the input string just before the last character you added to the output.
Repeat until you meet the null in the input.
Print the output string from the last position.

Try it on paper, it's a lot simpler than your version!

another way is to reverse the half length computed in integers. That does round down and so in the uneven case the middlest char isnt touched.

int evenHalf = 42 / 2; //21
int unevenHalf = 43 / 2; // also 21