-4

I am struggling to write a char* passed as an argument. I want to write some string to char* from the function write_char(). With the below code, I am getting a segmentation fault.

#include <stdio.h>
#include <string.h>
#include <stdlib.h>

void write_char(char* c){
    c = (char*)malloc(11*(sizeof(char)));
    c = "some string";
}

int main(){
    char* test_char;
    write_char(test_char);
    printf("%s", test_char);

    return 0;
}
Vlad from Moscow
  • 301,070
  • 26
  • 186
  • 335
Indri
  • 41
  • 1
  • 6
  • 6
    `c = malloc(...); c = "some string"` is somewhat similar to `x = 5; x = 7`. The second assignment overwrites the first. In this case, that causes a memory leak. Also, you're assigning to a variable scoped to the function, so the value is lost when the function returns. – William Pursell Sep 24 '19 at 13:43
  • 2
    Note that `"some string"` requires 12 characters — you didn't count the trailing null. But you should avoid hardwiring string lengths into the code. And writing out of bounds of the allocated array (if you copied the string to the allocated array, rather than leaking the memory) would be undefined behaviour. – Jonathan Leffler Sep 24 '19 at 14:01
  • You could use the `strdup`, that way you don't have to worry about null terminating your string. – ZeppRock Sep 24 '19 at 14:13

5 Answers5

8

You have two problems (related to what you try to do, there are other problems as well):

  1. Arguments in C are passed by value, which means that the argument variable (c in your write_char function) is a copy of the value from test_char in the main function. Modifying this copy (like assigning to it) will only change the local variables value and not the original variables value.

  2. Assigning to a variable a second time overwrites the current value in the variable. If you do e.g.

    int a;
    a = 5;
    a = 10;
    

    you would (hopefully) not wonder why the value of a was changed to 10 in the second assignment. That a variable is a pointer doesn't change that semantic.


Now how to solve your problem... The first problem could be easily solved by making the function return a pointer instead. And the second problem could be solved by copying the string into the memory instead of reassigning the pointer.

So my suggestion is that you write the function something like

char *get_string(void)
{
    char *ptr = malloc(strlen("some string") + 1);  // Allocate memory, +1 for terminator
    strcpy(ptr, "some string");  // Copy some data into the allocated memory
    return ptr;  // Return the pointer
}

This could then be used as

char *test_string = get_string();
printf("My string is %s\n", test_string);
free(test_string);  // Remember to free the memory we have allocated
Some programmer dude
  • 400,186
  • 35
  • 402
  • 621
  • 1
    You could use `char *ptr = malloc(sizeof("some string")); if (ptr != NULL) strcpy(ptr, "some string"); return ptr;` since [`sizeof("string literal")`](https://stackoverflow.com/questions/1392200/sizeof-string-literal) includes the null byte in the count, unlike `strlen()`. – Jonathan Leffler Sep 24 '19 at 14:41
2

Within the function

void write_char(char* c){
    c = (char*)malloc(11*(sizeof(char)));
    c = "some string";
}

the parameter c is a local variable of the function. Changing it within the function does not influence on the original argument because it is passed by value. That is the function deals with a copy of the original argument.

You have to pass the argument by reference through pointer to it.

Also the function has a memory leak because at first the pointer was assigned with the address of the allocated memory and then reassigned with the address of the first character of the string literal "some string".

If you want to create a copy of a string literal then what you need is the following

#include <stdio.h>
#include <string.h>
#include <stdlib.h>

void write_char( char **s )
{
    const char *literal = "some string";
    *s = malloc( strlen( literal ) + 1 );

    if ( *s ) strcpy( *s, literal );
}

int main( void )
{
    char *test_char = NULL;

    write_char( &test_char );

    if ( test_char ) puts( test_char );

    free( test_char );
}    

The program output is

some string

Do not forget to allocate dynamically a character array that is large enough to store also the terminating zero of the string literal.

And you should free the allocated memory when the allocated array is not needed any more.

If you want just to initialize a pointer with the address of a string literal then there is no need to allocate dynamically memory.

You can write

#include <stdio.h>

void write_char( char **s )
{
    *s = "some string";
}

int main( void )
{
    char *test_char = NULL;

    write_char( &test_char );

    puts( test_char );
}    
Vlad from Moscow
  • 301,070
  • 26
  • 186
  • 335
-1

In C, you'll need to pass a pointer to a pointer. Your malloc call is trying to change the value of the variable that's being passed in, but it's actually only a copy. The real variable you pass in will not be changed.

Also, the way that you copy a string into a char* is not using assignment... Here's some revised code:

#include<stdio.h>
#include<string.h>
#include<stdlib.h>

void write_char(char** c){
    size_t len = strlen("some string");
    *c = (char*)malloc(len + 1); // + 1 for null termination
    strncpy(*c, "some string", len);
}

int main(){
    char* test_char;
    write_char(&test_char);
    printf("%s", test_char);

    return 0;
}
jwismar
  • 12,164
  • 3
  • 32
  • 44
  • 3
    Why `strncpy()`? You've carefully avoided setting the null byte which you allocated space for (so what you create is not reliably a string). Using `strcpy(*c, "some string");` is perfectly safe; if you must use `strncpy()`, then `strncpy(*c, "some string", len + 1);` would be correct and safe. You could use `memmov(*c, "some string", len + 1);` (or `memcpy()` instead of `memmove()`) too. – Jonathan Leffler Sep 24 '19 at 13:58
  • @JonathanLeffler Note: "`strcpy(*c, "some string");` is perfectly safe" --> all the various copy functions suffer here if `*c == NULL`. – chux - Reinstate Monica Sep 24 '19 at 14:04
  • @JonathanLeffler I would argue that using `strncpy` is just a good muscle memory, and should not be discouraged. Obviously, it has to be used with correct arguments. – SergeyA Sep 24 '19 at 14:10
  • You're right, @chux — the code doesn't test that the allocation succeeded. In the context where the allocation succeeds, the `strcpy()` is perfectly safe. – Jonathan Leffler Sep 24 '19 at 14:12
  • @SergeyA `strncpy(*c, "some string", len);` is certainly wrong here an `*c` does not point to a _string_ : no _null character_. JL comment about needing a +1 is critical here. – chux - Reinstate Monica Sep 24 '19 at 14:13
  • @SergeyA — interesting; I'd head in the direction of "don't use `strncpy()` unless you need its null-padding and know that the string to be copied is smaller than the space it is copied into". That means you never use its 'truncating' property. I'd use `memmove()` more happily; I know how long the source string is, and how long the destination is, and therefore can compute the correct length to use. But YMMV. – Jonathan Leffler Sep 24 '19 at 14:14
  • @chux first off, I am taking back my comment re: null pointer (I misread *c for **c). Second off, I agree that `strncpy` was called with incorrect argument here, as I mentioned in my comment. The only thing I am highlighting is that one should not focus on using `strcpy` instead - rather just use `strncpy` with correct argument. – SergeyA Sep 24 '19 at 14:15
  • @JonathanLeffler I am strong believer that while `strncpy` is indeed flawed (https://randomascii.wordpress.com/2013/04/03/stop-using-strncpy-already/), but the fact that it makes developer to be conscious of size of the buffer is extremely valuable. Too many buffer overruns. – SergeyA Sep 24 '19 at 14:18
-3

String assignment in C is very different from most modern languages. If you declare a char * and assign a string in the same statement, e.g.,

char *c = "some string";

that works fine, as the compiler can decide how much memory to allocate for that string. After that, though, you mostly shouldn't change the value of the string with =, as this use is mostly for a constant string. If you want to make that especially clear, declare it with const. You'll need to use strcpy. Even then, you'll want to stay away from declaring most strings with a set string, like I have above, if you're planning on changing it. Here is an example of this:

char *c;
c = malloc(16 * sizeof(char));
strcpy(c, "Hello, world\n");

If you're passing a pointer to a function that will reallocate it, or even malloc in the first place, you'll need a pointer to a pointer, otherwise the string in main will not get changed.

void myfunc(char **c) {
    char *tmp = realloc(*c, 32 * sizeof(char));
    if(tmp != NULL) {
        *c = tmp;
    }
}

char *c = malloc(16 * sizeof(char));
strcpy(c, "Hello, world\n");
myfunc(&c);
anonmess
  • 465
  • 3
  • 9
  • 2
    I disagree with _"After that, though, you can't change the value of the string with `=`"._ When you have `char *c`, you can at any time change the string it points to by `c = "another string";` — which changes the pointer stored in the pointer variable. You shouldn't modify string literals; most often, attempting to do so leads to a crash. If you had `char c[] = "some string";` then you could not change the string by assignment. – Jonathan Leffler Sep 24 '19 at 13:55
  • If `tmp == NULL`, `*c` is not re-assigned. Calling codes ability to know that `realloc()` failed. – chux - Reinstate Monica Sep 24 '19 at 13:58
  • @chux yeah it's not foolproof code or anything, just using it to demonstrate. – anonmess Sep 24 '19 at 13:59
  • `char *c = "some string";` is certainly **not** fine, as you initialize non-const char pointer with a char literal. – SergeyA Sep 24 '19 at 13:59
  • 2
    @SergeyA maybe not best practice, but certainly fairly common usage. – anonmess Sep 24 '19 at 14:02
  • Being a "common usage" doesn't mean it's fine. Good answer should reflect this. – SergeyA Sep 24 '19 at 14:06
  • @SergeyA Good comments would use _string literal_ for `"some string"`. C does not define _char literal_. – chux - Reinstate Monica Sep 24 '19 at 14:08
  • @chux yeah, it was a typo. Traditional, quality of comments is lower than quality of answers (because you can't edit them after 5 mins). Also, C most certainly **does** define character literal (char being an abbreviation of character) – SergeyA Sep 24 '19 at 14:12
  • @SergeyA C does not define character literal. C has _string literals_ and _compound literals_. Perhaps you could cite C's character literal definition? – chux - Reinstate Monica Sep 24 '19 at 14:15
  • @chux oh wow! I stand corrected. It is only C++ which defines it, C has character constant... I learned something today. – SergeyA Sep 24 '19 at 14:21
  • @SergeyA (& chux): C11 [§6.4.4.4 Character constants](https://port70.net/~nsz/c/c11/n1570.html#6.4.4.4) applies to `'a'` etc — an _integer character constant_. And [§6.4.5 String literals](https://port70.net/~nsz/c/c11/n1570.html#6.4.5). Yes, it's another place where C and C++ diverge. – Jonathan Leffler Sep 24 '19 at 14:22
  • @SergeyA Tip: C _literals_ can have their address taken. Not so with constants. – chux - Reinstate Monica Sep 24 '19 at 14:25
-4
char* test_char="string"; // initialize string at the time of declaration


void write_char(char* c){
    c = (char*)malloc(11*(sizeof(char)));

}

int main(){
    char* test_char="strin";
    write_char(test_char);
    printf("%s", test_char);

    return 0;
}