Passing strings to a struct in C

Question

I'm trying to do something simple in C that is passing 2 names (from argv[]) to a structure. I feel like I'm all over the place with this. This is my code:

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

typedef struct
{
    char *name1;
    char *name2;

}names;

void writeNames(names* c ,char n1[], char n2[]){
    char* buff;

    buff = malloc(strlen(n1)*sizeof(char)+1);
    strcpy(buff, n1);
    c->name1 = buff;
    free(buff);
    buff = NULL;

    buff = malloc(strlen(n2)*sizeof(char)+1);
    strcpy(buff, n2);
    c->name2 = buff;
    free(buff);
    buff = NULL;
}

int main(int argc, char const *argv[])
{
    names card;
    writeNames(&card,argv[1],argv[2]);
    printf("%s %s\n",card.name1,card.name2);
    return 0;
}

and this is what I get:

naming.c: In function ‘main’:
naming.c:31:2: warning: passing argument 2 of ‘writeNames’ discards ‘const’ qualifier from pointer target type [enabled by default]
  writeNames(&card,argv[1],argv[2]);
  ^
naming.c:12:6: note: expected ‘char *’ but argument is of type ‘const char *’
 void writeNames(names* c ,char n1[], char n2[]){
      ^
naming.c:31:2: warning: passing argument 3 of ‘writeNames’ discards ‘const’ qualifier from pointer target type [enabled by default]
  writeNames(&card,argv[1],argv[2]);
  ^
naming.c:12:6: note: expected ‘char *’ but argument is of type ‘const char *’
 void writeNames(names* c ,char n1[], char n2[]){
      ^

I don't really undertand what is going on.

Answer 1

c->name1 = buff;

After this line, c->name1 and buff have the same value.

free(buff);

Since c->name1 and buff are equal, this is equivalent to free(c->name1), which clearly is not what you want.

Also, change

void writeNames(names* c ,char n1[], char n2[]){

to

void writeNames(names* c ,char const n1[], char const n2[]){

Answer 2

the posted code:

void writeNames(names* c ,char n1[], char n2[]){
    char* buff;

    buff = malloc(strlen(n1)*sizeof(char)+1);
    strcpy(buff, n1);
    c->name1 = buff;
    free(buff);
    buff = NULL;

    buff = malloc(strlen(n2)*sizeof(char)+1);
    strcpy(buff, n2);
    c->name2 = buff;
    free(buff);
    buff = NULL;
}

is nonsence.

The reason is that c->name1 and c->name2 are pointers, but not pointed to anything in particular.

This line:

c->name2 = buff;

copies the address of 'buff' to the first address of c->name2

That is not what seems to be wanted.

always check for system errors

always cleanup memory allocations before exiting

sizeof( char ) is always 1 so has no effect on the malloc

suggest:

void writeNames(names* c ,char n1[], char n2[])
{
    c->name1 = malloc(strlen(n1) +1);
    if( NULL == c->name1 )
    { // then, malloc failed
        perror( "malloc for name1 failed");
        exit( EXIT_FAILURE );
    }

    // implied else, malloc successful

    strcpy(c->name1, n1);


    c->name2 = malloc(strlen(n2) +1);
    if( NULL == c->name2 )
    { // then, malloc failed
        perror( "malloc for name2 failed");
        free( c->name1 );
        exit( EXIT_FAILURE );
    }

    // implied else, malloc successful

    strcpy(c->name2, n2);
} // end function: writeNames