Remix.run Logo
▲ asveikau 2 days ago

Just from the first example:

    struct user *user_new(const char *name, size_t n_groups)
    {
        struct user *u = malloc(sizeof *u);
        if (u == NULL)
            goto fail;
        u->name = strdup(name);
        if (u->name == NULL)
            goto fail_user;
        u->groups = calloc(n_groups, sizeof *u->groups);
        if (u->groups == NULL)
            goto fail_name;
        return u;

    fail_name:
        free(u->name);
    fail_user:
        free(u);
    fail:
        return NULL;
    }
I think you can represent C better than this. I like to focus on one cleanup block, and exploit the fact that free(NULL) is a no-op.

    struct user *user_new(const char *name, size_t n_groups)
    {
        struct user *u = calloc(sizeof *u, 1);
        if (u == NULL)
            goto fail;
        u->name = strdup(name);
        if (u->name == NULL)
            goto fail;
        u->groups = calloc(n_groups, sizeof *u->groups);
        if (u->groups == NULL)
            goto fail;
        return u;
    fail:
        if (u)
        {
            free(u->name);
            free(u->groups);
        }
        free(u);
        return NULL;
    }
Although note that this assumes calloc(sizeof *u, 1) initializes pointers to NULL, IIRC assuming the bit pattern of NULL is all zeroes is technically against the standard but every system you're likely to target will still work.

If you follow the pattern of "it's OK to free NULL pointers", you can also re-use a free function:

    struct user *user_new(const char *name, size_t n_groups)
    {
        struct user *u = calloc(sizeof *u, 1);
        if (u == NULL)
            goto fail;
        u->name = strdup(name);
        if (u->name == NULL)
            goto fail;
        u->groups = calloc(n_groups, sizeof *u->groups);
        if (u->groups == NULL)
            goto fail;
        return u;
    fail:
        user_free(u);
        return NULL;
    }

    void user_free(struct user *u)
    {
        if (u)
        {
            free(u->name);
            free(u->groups);
        }
        free(u);
    }
Then user_free() can also be a user-facing API function for cleanup, used elsewhere, and it can be written to be cool with tearing down partially initialized structures.