Correct memory allocation for a struct

Question

Having a struct defined in a such way, I need to allocate memory

typedef struct string_collection {
    char **c;
    size_t current, allocated;
} TSC, *ASC;

So I came with this code, is it right or I missed something? First allocating struct descriptor and then enough space for d pointers to string

ASC AlocSC(size_t d)
{
    ASC sc;

    sc = (TSC*) malloc(sizeof(TSC));
    if (!sc) return NULL;

    sc->c = calloc(d, sizeof(char *));

    if (!sc->c) {
        free(sc);
        return NULL;
    }

    sc->current = 0;
    sc->allocated = d;

    return sc;
}

Are “x” and “sc” meant to have been the same thing? – BRPocock Dec 08 '11 at 21:22 — BRPocock, Dec 08 '11 at 21:22
@MikeEgren: You forgot one more `x`. – AusCBloke Dec 08 '11 at 21:26 — AusCBloke, Dec 08 '11 at 21:26

score 2 · Answer 1 · edited May 23 '17 at 11:48

2

As long as x is replaced with sc, it looks ok to me. You shouldn't, however, cast the return of malloc in C (read more here). I would instead have for that line:

sc = malloc(sizeof(*sc));

You can do the same for the size of type x->c points to (char*).

edited May 23 '17 at 11:48

Community

1
1

answered Dec 08 '11 at 21:24

AusCBloke

18,014
6
40
44

3

and, as a stylistic thing: if you're going to typedef a “pointer type,” perhaps use it consistently. Personally, I dislike pointer typedefs as I think they're confusing to others reading the code; but it's more confusing still to see `TSC*` and `ASC` both being used; one has to retreat to the header file to discover that they're the same thing. – BRPocock Dec 08 '11 at 21:27
@BRPocock: Yeah exactly right, it irritates me to have *simple* pointer types `typedef`'d, but since I took out `(TSC*)` I thought I'd skip the point of using `(ASC)` instead. – AusCBloke Dec 08 '11 at 21:30
That was more directed at the original poster … in hopes that I don't wind up maintaining/editing/replacing yet another program full of obscured pointer types. I have a hard enough time remembering my `->` and `.` dereferences *with* all the `*`/`&` hints in C… – BRPocock Dec 08 '11 at 21:33

Greg Jandl · Accepted Answer · 2011-12-08T21:32:53.257

2

The code as edited is essentially correct, though I have several stylistic differences with you (such as not doing a typedef to hide the "pointerness" of an object, not using the size of the allocated object in the malloc/calloc call, and a few other things).

Your code, "cleaned up" a bit:

TSC *AlocSC(size_t d)
{
    TSC *sc = malloc(sizeof *sc);
    if (!sc) return NULL;

    sc->c = calloc(d, sizeof *sc->c);
    if (!sc->c) {
        free(sc);
        return NULL;
    }

    sc->current = 0;
    sc->allocated = d;

    return sc;
}

edited Dec 08 '11 at 21:32

answered Dec 08 '11 at 21:26

Greg Jandl

831
9
12

I think he had it right, actually. His sc->c element is a `char**` – BRPocock Dec 08 '11 at 21:28
Why sizeof(char) and not sizeof(char*) ? – Mike Egren Dec 08 '11 at 21:29
@MikeEgren: You had it right, you want an array of string pointers and that's what you allocated. – AusCBloke Dec 08 '11 at 21:30
Ach ... missed that it was char **; I had assumed a string. Will edit answer accordingly. – Greg Jandl Dec 08 '11 at 21:32
If the type is completely opaque (incomplete in the header used by the consumer/programmer), then there's no harm in hiding the pointer-ness. Otherwise, I fully agree. – Jonathan Leffler Dec 08 '11 at 21:36
@Jonathan Leffler - I concur. Of course, in that case you also need to go to some effort to allow a user to be able to allocate instances of that object themselves. Just a size is insufficient, since it doesn't take alignment requirements into account. – Greg Jandl Dec 08 '11 at 21:42
3

Note that using `calloc()` to allocate an array of pointers is not guaranteed to set those pointers to `NULL`. The language does not guarantee that a null pointer is represented as all-bits-zero. (Most compilers do so, but it's not a good idea to depend on it.) Recommendation: allocate the array of pointers with `malloc()`, and don't access any of the pointers until you've explicitly assigned a value to them. – Keith Thompson Dec 08 '11 at 21:50
Since the compiler can determine the size of the pointer to an incomplete type, it isn't crucial that the user know the size; indeed, if the user must know the size of the structure behind the opaque type, it isn't a completely opaque type. Granted, though, that means that the system will have to allocate the memory, which can sometimes be a problem. – Jonathan Leffler Dec 08 '11 at 23:33
@Jonathan Leffler - sounds like we're violently agreeing - as long as the user is OK with heap-only allocation of the objects by a library function, then of course there's no need for the user to have any idea of the size of the object. – Greg Jandl Dec 09 '11 at 04:46

Correct memory allocation for a struct

2 Answers2