-4

Here are the codes, it's C code. And please explain return value of ="string"

    char * p = (char*) malloc(sizeof(char) * 100);
    p = "hello";   
    *(p+1) = '1';
    printf("%s", p);
    free(p);
Mooing Duck
  • 64,318
  • 19
  • 100
  • 158
Yutao Xing
  • 41
  • 5
  • 1
    you're trying to write to a write-only address because the string "hello" is in your .ro data section. Furthermore, what do you want to achieve with this code? – Evert Feb 26 '15 at 21:15
  • 1
    1) You have memory leak since you lose the pointer `p` returned by `malloc()` 2) you are trying to modify a string literal - that is UB. – P.P Feb 26 '15 at 21:16
  • 1
    Line 3: [Why do I get a segmentation fault when writing to a string?](http://stackoverflow.com/q/164194/33499) – wimh Feb 26 '15 at 21:16
  • What makes you think something is wrong with it? – Scott Hunter Feb 26 '15 at 21:16
  • 1
    and you are freeing a ro memory section. too many problems. – Jason Hu Feb 26 '15 at 21:17

6 Answers6

6

Notice that p = "hello"; does not copy any string, but just sets the pointer p to become the address of the 6 bytes literal string "hello". To copy a string use strncpy or strcpy (read strncpy(3) ...) but be scared of buffer overflows.

So you do

 char * p = malloc(100);

which allocates a memory zone capable of holding 100 bytes. Let's pretend that malloc succeeded (read malloc(3)...) and returned the address 0x123456 for example (often, that concrete address is not reproducible from one run to the next, e.g. because of ASLR).

Then you assign p = "hello"; so you forgot the address 0x123456 (you've got now a memory leak), and you put in p the address of the 6 bytes literal string "hello" (let's imagine it is 0x2468a).

Later the machine executes the code for *(p+1) = '1'; so you are trying to replace the e character (at address 0x2468b) inside literal "hello" by 1. You get a segmentation violation (or some other undefined behavior), since that literal string sits in constant read only memory (e.g. the text segment of your executable, at least on Linux).

A better code might be:

 #define MY_BUFFER_LEN 100
 char *p = malloc(MY_BUFFER_LEN);
 if (!p) { perror("malloc for p"); exit(EXIT_FAILURE); };
 strncpy(p, "hello", MY_BUFFER_LEN); /// you could do strcpy(p, "hello")
 *(p+1) = '1';
 printf("%s", p);
 free(p);

and if you are lucky (no memory failure) that would much later output h1llo (the output will happen only when stdout becomes flushed since it is buffered, e.g. by some later call to fflush). So don't forget to call

 fflush(NULL);

after the previous code chunk. Read perror(3), fflush(3).

A generic advice is to read documentation of every function that you are using (even printf(3) ...).

Regarding printf and related functions, since stdout is often line buffered, in practice I strongly recommend to end every format control string with \n -e.g. printf("%s\n", p); in your code; when you don't do that (there are some cases where you don't want to...) think twice and perhaps comment your code.

Don't forget to compile with all warnings and debug info (e.g. gcc -Wall -Wextra -g) then learn how to use the debugger (e.g. gdb)

Community
  • 1
  • 1
Basile Starynkevitch
  • 223,805
  • 18
  • 296
  • 547
2

Your code doesn't do what you think it does.

"hello" is behind the scenes a pointer to a static array of six chars, most likely write protected.

When you assign it to p, the pointer that malloc returned is lost, instead p now contains a pointer to that static array of six chars.

The assigment to p+1 may crash, or may not crash, but whatever it does, it is undefined behaviour and will cause trouble.

free (p) tries to free a static array of six chars. That isn't going to work. Again, undefined behaviour, and an immediate crash if you are lucky.

gnasher729
  • 51,477
  • 5
  • 75
  • 98
1

There are problems in nearly each line:
1) In the first line you are assigning to p the address of newly allocated memory. which is OK
2) In the next line you overwrite it with an address of some static string. Which is bad, since the allocated memory is "lost", thus causing memory leak.
3) In the third line you are trying to overwrite something in the static string location, which might be read only, which is bad.
4) In the last line you are trying to free the memory at the string's location, which is memory violation.

Eugene Sh.
  • 17,802
  • 8
  • 40
  • 61
0

1) In C it is incorrect cast the return of malloc() (C++, okay to cast)
2) Once allocated memory, assigning a value to p is not done by =.
strcpy(p, "hello"); is better.
3) Once assigned correctly, using strcpy(), *(p+1) = '1' is the same as p[1] = '1', and will result in "h1llo".

By the way, using the line *(p+1) = '1' AFTER using p = "hello" to assign the value hello (instead of strcpy()) will cause problems, as described by @gnasher, @Basile and others.

ryyker
  • 22,849
  • 3
  • 43
  • 87
0

You need to understand what pointers are.

In C, there is no variable that can store a string. Instead the strings are stored in arrays of chars. In order to be able to handle such an array, you use a variable (called pointer) that stores the memory address of the first element of the array. Also, strings have a special character (the nul character) the indicate where they end, but that's not relevant here.

In the code you posted, you allocate memory to store 100 chars (this is usually 100 bytes) and get a pointer to the first element of this memory region, called p (for further reading: Do I cast the result of malloc?).

Whenever you use a string literal, some memory gets allocated and it gets stored in that memory region. When you assign it to p, p now points to the first element of the new memory region, that stores the string (so you've lost the memory allocated you malloc -> memory leak). Now, you try to modify the string pointed at by p. This won't work, as the string literals are stored as constant strings. And after that, you try to free the memory of this string, which is not possible. All of these mistakes generate runtime errors and are not very easy to detect and fix. In order to be able to achieve what you want, use a function like strcpy to copy the string from the read-only memory region to your pointer:

strcpy(p, "string");
Community
  • 1
  • 1
Paul92
  • 8,827
  • 1
  • 23
  • 37
0

Strings in C are arrays, and you can't use = to assign values to arrays, only to indexed elements of arrays. For copying string data, use strcpy() or strncpy(). Your code could look like:

char *p = (char*) malloc(sizeof(char) * 100);
strcpy(p, "hello");   
*(p+1) = '1';
printf("%s", p);
free(p);

Try that. The difference is that p = "hello" will set the pointer p to point to the constant string "hello", overwriting the pointer to the 100-byte memory block you just allocated. The free(p); call below will fail because you're passing a pointer not returned by an allocation function, even if the system you're using doesn't give a write fault on attempting to modify constant data.

If you ever do need to point a pointer at a constant string (it happens), be sure you declare the pointer as const char *. This is required in C++. I don't know if it's required in newer versions of C, but it's a Real Good Idea in any case.

Mike Housky
  • 3,959
  • 1
  • 17
  • 31