0

Why this code won't work, on online compiler return segment fault, on my linux VPS memory leak...

#include <ctype.h>
#include <stdio.h>
#include <string.h>
char *a_foo(char *str) {
    unsigned char *p = (unsigned char *)str;

    while (*p) {
        *p = 'a';
        p++;
    }
    return str;
}

int main() {
    char * test = "TestTest";
    a_foo(test);
    printf("result: %s\n", test);
}

Online Compiler: LINK

Łukasz Mleczko
  • 173
  • 2
  • 12
  • 1
    Because `test` is a read only string and you are trying to modify it. Change to `char test[] = "TestTest";` – kaylum Oct 06 '16 at 23:38
  • Also, because you ignored all the warning messages from your compiler. – Sam Varshavchik Oct 06 '16 at 23:39
  • 1
    @SamVarshavchik: What warning messages? I get none at all with `gcc -std=c11 -pedantic -Wall -Wextra`. In C, string literals are not `const`, but attempting to modify them has undefined behavior. – Keith Thompson Oct 06 '16 at 23:41
  • Why is this tagged both C and C++? They're two different languages, and though they're closely related some of the differences are relevant to your code. – Keith Thompson Oct 06 '16 at 23:43
  • @KeithThompson Culprit possibly is the hard way casting here: _`unsigned char *p = (unsigned char *)str;`_ – πάντα ῥεῖ Oct 06 '16 at 23:44
  • Because this don't working in C and C++. I want why in C and why in C++ .. Yes I know that C and C++ its very different – Łukasz Mleczko Oct 06 '16 at 23:45
  • @πάνταῥεῖ: No, that cast is questionable, but it shouldn't cause any problems. There's no point in making `p` an `unsigned char*`. Just drop the cast and define `char *p = str;`. The cast might cause undefined behavior but in practice it's not likely to cause any visible symptoms. – Keith Thompson Oct 06 '16 at 23:46
  • Then please make that clear in your question. I suggest limiting this one to C; if there are any C++-specific issues remaining after it's answered, consider posting a new question. In any case, I agree with @kaylum that this is a duplicate. – Keith Thompson Oct 06 '16 at 23:47
  • @KeithThompson Changed :) – Łukasz Mleczko Oct 06 '16 at 23:55
  • 1
    don't use literal string but use for example an array of char `char test[] = "TestTest"; ` – Raindrop7 Oct 06 '16 at 23:58

1 Answers1

2

The string literal "TestTest" is probably stored in read-only memory in your environment, so the code in a_foo that attempts to write to it would fail.

The type of a string literal has the const qualifier and the compiler should warn you if you try to assign it to a non-const pointer variable.

David Grayson
  • 84,103
  • 24
  • 152
  • 189
  • Thanks You, now I understand :) – Łukasz Mleczko Oct 06 '16 at 23:41
  • No, the type of a C string literal is `char[N]` (where `N` is the length plus 1 for the terminating null character), and the type of a C++ string literal is `const char[N]`. Arrays are not pointers. – Keith Thompson Oct 06 '16 at 23:42
  • it would fail? define "fail"? fail to compie? – Ryan Oct 06 '16 at 23:43
  • @self: What do you mean? Attempting to modify a string literal has undefined behavior, and is likely to cause a set fault or something similar depending on the implementation. – Keith Thompson Oct 06 '16 at 23:44
  • exactly it is undefined behavior, that means anything can happen including it not failing. So you can't say this will fail – Ryan Oct 06 '16 at 23:45
  • 1
    @self: See the word "probably" earlier in that sentence. The literal is *probably* stored in read-only memory; if it is, the attempt to write to it will fail. (That's read-only memory in the sense that it's marked as read-only by the OS, not physical ROM.) If you assume that the "probably" only applies to the first half of the sentence, I suppose I can see your point, but there is a reasonable interpretation. – Keith Thompson Oct 06 '16 at 23:48
  • Yeah, given that the OP is getting a segmentation fault, the OP's OS/compiler/runtime environment probably was storing the string in read-only memory which is protected at runtime from being modified, and writes will fail. – David Grayson Oct 07 '16 at 02:54
  • Thanks for the correction in your first comment, Keith. – David Grayson Oct 07 '16 at 02:55