I'm learning pointers in C and i came across a confusion between pointers X struts X functions

The goal: creating two structs and mutate properties inside them.

The path I'm going: I am creating these two structs and then passing its memory addresses to the mutate function, the function then prints and mutates some properties of these structs.

Result I get:

1: The name of the struct created is nod being entirely printed and its of the wrong struct passed, and the life property is not properly changed and printed to the screen.

2: On the terminal I get "Segmentation Fault", not sure why but I'm pretty sure its something wrong I did.

Here's my code:

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

typedef struct {
    int power;
    int life;
    char name[];
} Hero;

void attackHero(Hero *hero, int *power) {

    (*hero).life = (*hero).life - *power;

    printf("Damage: %d\n", *power);
    printf("Attacked hero: %s\n", (*hero).name);
    printf("Hero's remaining life: %d\n", (*hero).life);

};

int main () {
    Hero flash;
    flash.power = 250;
    flash.life = 500;
    strcpy(flash.name, "The Flash");
    
    Hero batman;
    batman.power = 380;
    batman.life = 700;
    strcpy(batman.name, "Batman arkham knight");

    attackHero(&flash, &batman.power);

    return 0;
}

Result printed to the terminal (Vscode gcc):

CodePudding user response：

Here is the warning that I get when I compile your original code:

1.c:25:2: warning: ‘__builtin_memcpy’ writing 10 bytes into a region of size 0 overflows the destination [-Wstringop-overflow=]
   25 |  strcpy(flash.name, "The Flash");
      |  ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
1.c:30:2: warning: ‘__builtin_memcpy’ writing 21 bytes into a region of size 0 overflows the destination [-Wstringop-overflow=]
   30 |  strcpy(batman.name, "Batman arkham knight");
      |  ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

If you want to use the flexible array then you have to allocate space for it like this:

int main () {
    Hero *flash = malloc(sizeof(*flash)   sizeof("The Flash"));
    flash->power = 250;
    flash->life = 500;
    strcpy(flash->name, "The Flash");

    Hero *batman = malloc(sizeof(*flash)   sizeof("Batman arkham knight"));
    batman->power = 380;
    batman->life = 700;
    strcpy(batman->name, "Batman arkham knight");
    attackHero(flash, &batman->power);

    free(flash);
    free(batman);
}

Here there the resulting code refactored a bit, and I added a error check for malloc:

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

typedef struct {
    int power;
    int life;
    char name[];
} Hero;

Hero *createHero(int power, int life, const char *name) {
    Hero *h = malloc(sizeof(*h)   strlen(name)   1);
    if(!h) {
       printf("malloc failed\n");
       exit(1);
    }
    h->power = power;
    h->life = life;
    strcpy(h->name, name);
    return h;
}

void attackHero(Hero *hero, int power) {
    hero->life -= power;
    printf(
        "Damage: %d\n"
        "Attacked hero: %s\n"
        "Hero's remaining life: %d\n",
        power,
        hero->name,
        hero->life
    );
};

int main(void) {
    Hero *flash = createHero(250, 500, "The Flash");
    Hero *batman = createHero(380, 700, "Batman arkham knight");
    attackHero(flash, batman->power);
    free(flash);
    free(batman);
}

Alternatively use a fixed array (char [64] as suggested by @Diego) or a char * and allocate space to it. The former only needs 2 lines of code change from the original:

// largest name in use
#define NAME_LEN sizeof("Batman arkham knight")

typedef struct {
    int power;
    int life;
    char name[NAME_LEN];
} Hero;

CodePudding user response：

Whole lotta malloc() going on. Since the hero’s names are string literals (and assuming they don’t change), just change name[]; to const char *name in the structure and initialize via simple assignment:

flash.name = "The Flash";
batman.name = "Batman arkham knight";

No worries about malloc() failures, name sizes or free() requirements.