0

I want to reverse a string, but I got a error on swap char.

Could someone help me with this?

char*  reverse_char(char* src){
    char *p,*q;
    p = q = src;
    while(*(p) != '\0'){
        p++;
    }
    p--;

    char  temp = '\0';
    while (p > q) {
        temp = *p;
        *p = *q; // I got exec bad access here ??? why 
        *q = temp;
        p--;
        q++;
    }
    return src;
}

This is the main method.

int main(int argc, char const* argv[])
{
    char *hi = "hello world!\n";
    printf("%s", hi);

    reverse_char(hi);
    printf("%s", hi);
    return 0;
}
Owen Versteeg
  • 342
  • 2
  • 14
sharewind
  • 2,299
  • 4
  • 16
  • 12
  • Hint: `char *hi` is really `const char *hi` - in your function, you are trying to overwrite read only memory – Andreas Fester Nov 05 '14 at 08:06
  • All that is used between `""` is named a "string literal" String literals in C are stored in read only memory. What this is affecting you can read here: http://stackoverflow.com/a/18416748/2003898 – dhein Nov 05 '14 at 08:08
  • In function `main`, variable `hi` is pointing to a string which resides in a read-only memory segment. In function `reverse_char`, you are attempting to change the contents of that string, hence the memory access violation. – barak manos Nov 05 '14 at 08:08
  • Related: http://stackoverflow.com/questions/2245664/string-literals-in-c?lq=1 and http://stackoverflow.com/questions/2589949/c-string-literals-where-do-they-go?lq=1 – Jason C Nov 05 '14 at 08:15

2 Answers2

6

Replace char *hi = "hello world!\n"; with char hi[] = "hello world!\n";

"hello world!\n" is string literal and may not be writable causing the access error. Modifying the contents of string literal is undefined behavior, it may write the contents silently, raise an access error or may do something else unexpected. (So you should not write to string literal)

Summing up

char a[] = "...";  /* Contents of a can be assigned */
char *a = "...";   /* Writing to contents of memory pointed by a leads to UB */
Mohit Jain
  • 30,259
  • 8
  • 73
  • 100
  • The first part of this answer is OK (although could use some extra info) but your bit at the end with the pointer assignments isn't really related to the OP's problem (or to the first part of your answer). – Jason C Nov 05 '14 at 08:11
  • @JasonC My point is, OP's intention falls in case 3 where writing to memory is bad. This data may lie in Read-Only memory. – Mohit Jain Nov 05 '14 at 08:13
  • But how is that related to whether or not you can use an array type on the left side of an assignment expression? – Jason C Nov 05 '14 at 08:16
  • Memory pointed by `&hi[i]` can not be at left hand side of assignment expression as assigning to it is bad. About having `hi` to the left hand side of assignment, I added it make case1 and case2 different. So that OP can choose the right declaration as per his requirements. – Mohit Jain Nov 05 '14 at 08:22
  • `&(hi[i])` is the same as `hi + i`, and `*(&(hi[i]))` and `*(hi + i)` both can certainly be assigned to if they are valid and not const... but whatever. I won't press the point because the rest of the answer is good. I believe your cases and breakdown add unnecessary noise to an otherwise good and clear answer. You have a +1 from me anyways. – Jason C Nov 05 '14 at 08:25
  • @JasonC Thanks, I would try to rephrase the answer again. – Mohit Jain Nov 05 '14 at 08:41
2

Though string literals in C have types of non-const character arrays they may not be changed.

From the C Standard (6.4.5 String literals)

7 It is unspecified whether these arrays are distinct provided their elements have the appropriate values. If the program attempts to modify such an array, the behavior is undefined.

it is better to declare a pointer initialized with a string literal as having type const char * as it is done in C++. For example

const char *hi = "hello world!\n";

In this case instead of a run-time error you would get a compile-time error that would be understandable because the parameter of the function is declared as a non-const character pointer.

You have to use a character array instead of a pointer to a string literal.

For example

char hi[] = "hello world!\n";

As for the function that it has some problem when a pointer points to an empty string. In this case after operation

p--;

you can get invalid pointer that is not necessary less than src.

I would write the function the following way

char*  reverse_string( char *s )
{
    char *p = s;

    while ( *p ) ++p;

    if ( p != s )
    {
        for ( char *q = s; q < --p; ++q )
        {
            char c = *q;
            *q = *p;
            *p = c;
        }
    }

    return s;
}
Vlad from Moscow
  • 301,070
  • 26
  • 186
  • 335