[SOLVED] [c] malloc with struct->char **variablelengtharray

Mol_Bolom · 05-01-2013, 03:15 PM

I've had a bad case of coders block for quite some time, and honestly, I'm just tired of searching, reading,reviewing,examing with gdb, cgdb, ddd...

Anyhoo, the idea goes something like this.

Code:

typedef struct SOME_KIND_OF_STRUCT
{
     // Any other data types I may want.
     int arraycount;
     struct SOME_KIND_OF_STRUCT *next;
     char **array;  // Purposefully at the END of the struct.
} SOME_KIND_OF_TYPE;

To which I could do...(see johnsfine's post #5 which is more concise than this)

Code:

SOME_KIND_OF_TYPE *skot;
skot = malloc(sizeof(SOME_KIND_OF_TYPE)+sizeof(size_t)*arraycount+sizeof(char)*listofstringslength);
strncpy((char *)(skot + sizeof(SOME_KIND_OF_TYPE) + sizeof(size_t) * arraycount), listofstrings, listofstringslength);
// Or memcpy if needs be...

Thusly, because char **array is at the end of the struct, and because the 'variable length array of pointers' is created at the end of the struct after the call to malloc.
Then I should be able to use/test skot->arraycount in a for loop to point to each string element stored at the end of the malloc'ed space for each possible skot->array[x] (Though, by looking at the code, it looks like it may create an extra pointer to which I could set to NULL for testing the end of skot->array[x] and wouldn't need skot->arraycount).

Sooooo! Yay or nay? Good or bad? Or was the implied question not explanatory enough? Or was it purely stupid of me to ask? Heh!

johnsfine · 05-01-2013, 03:41 PM

It is not correct to use char **array; for that.

I always forget which compilers accept
char *array[];
vs. which accept
char *array[0];
but one of those is correct for what you seem to be trying to do.

Quote:

Originally Posted by Mol_Bolom

Code:

malloc(sizeof(SOME_KIND_OF_TYPE)+sizeof(size_t)*arraycount+sizeof(char)*listofstringslength);

That part should work, but your intent would be a lot clearer if you had sizeof(char*) instead of sizeof(size_t).

After that, I'm lost on what you think you're doing, partially because I don't know what listofstrings is supposed to be.

strncpy stops at either the first '\0' (end of first string?) or the specified length, whichever comes first.

I think I understand what kind of struct you intend to create and what sort of contents it should have. But I don't see how you intend to populate those contents.

millgates · 05-01-2013, 04:18 PM

Quote:

Originally Posted by Mol_Bolom

Thusly, because char **array is at the end of the struct, and because the 'variable length array of pointers' is created at the end of the struct after the call to malloc.
Then I should be able to use/test skot->arraycount in a for loop to point to each string element stored at the end of the malloc'ed space for each possible skot->array[x] (Though, by looking at the code, it looks like it may create an extra pointer to which I could set to NULL for testing the end of skot->array[x] and wouldn't need skot->arraycount).

Sooooo! Yay or nay?

Nay.

Quote:

Originally Posted by Mol_Bolom

Good or bad?

Bad.

The usual way is to allocate the struct first:

Code:

skot = (SOME_KIND_OF_TYPE*)malloc(sizeof(SOME_KIND_OF_TYPE));

Then, you allocate the array of pointers:

Code:

skot->arraycount = arraycount;
skot->array = (char **)malloc(skot->arraycount * sizeof(char*));

And then, you allocate the individual strings:

Code:

for (size_t i = 0; i < skot->arraycount; i++) {
    skot->array[i] = (char *)malloc(size_of_the_strings[i] * sizeof(char));
}

Please note that the array of pointers and the strings will be allocated at "random" places in the memory, not necessarily at the end of the struct.

You can probably do it the way you attempted with some kind of very ugly hacks, (and you would also have to set each pointer in the struct to point somewhere), but I would not recommend this way unless you have a very good reason to do so and you know exactly what you're doing.

Mol_Bolom · 05-01-2013, 05:19 PM

Quote:

Originally Posted by johnsfire

I think I understand what kind of struct you intend to create and what sort of contents it should have. But I don't see how you intend to populate those contents.

<Edit> The text I wrote was horrible, and should have been explained as follows.

As long as malloc creates a contiguous space, and as long as the programmer takes into account the number and size of elements. Then she/he should be able to populate the pointers to the proper locations within the malloc'ed space.

</Edit>

Hopefully the next part will explain that (in a way that I happen to understand it).

Bad assembly representation below.

Code:

SOME_KIND_OF_STRUCT:
'   other data types I may use here.
arraycount:  resd 1
next:        resd 1         ' If memory serves resd is used for pointers.  Been awhile.
array[0]:    resd 1         ' First pointer of array that does not exist as of yet.
END_SOME_KIND_OF_STRUCT:    'Just for examples below

sizeof(SOME_KIND_OF_STRUCT) should be equivalent to END_SOME_KIND_OF_STRUCT - SOME_KIND_OF_STRUCT (in the poor assembly representation).

And after malloc (somwhere unbeknownst to the programmer in memory this space should be created).

Code:

skot:
'   Other data types.
skot->arraycount:  resd 1
skot->next:        resd 1
skot->array[0]:    resd 1           ' Again, I don't remember exactly, but I presume resd is for pointers.
skot->array[x]:    resd arraycount  ' for all x such that 0 < x <= arraycount
stringlistafterskot:   resb stringlistsize

I would think this would be correct, as long as malloc always creates a contiguous space in memory.
Then the position of the string list should be at sizeof(SOME_KIND_OF_STRUCT)+sizeof(char *)*arraycount (give or take 1).

Quote:

Originally Posted by johnsfire

After that, I'm lost on what you think you're doing, partially because I don't know what listofstrings is supposed to be.

strncpy stops at either the first '\0' (end of first string?) or the specified length, whichever comes first.

That's what I thought, but forgot. Which is why I had written "// Or memncpy if needs be" because I couldn't remember exactly.

Quote:

Originally Posted by millgates

The usual way is to allocate the struct first:

Then, you allocate the array of pointers:

And then, you allocate the individual strings:

That's what I had been doing, and can do so without any problems.

It just seems odd to re-call malloc to create a new space which should be found easily by adding, subtracting where in memory the malloc'ed struct was created.

Erf! I've lost my train of thought. If this doesn't help, I'll attempt it again later.

Anyway, thanks so far. You helped me with some things that have been 'blocking' me, and now know to stop using strncpy.

johnsfine · 05-01-2013, 06:13 PM

I think this is what you are trying to do (the parts in red are a test harness around the part you want).

Code:

#include <stdio.h>
#include <string.h>
#include <stdlib.h>
int main()
{
    int count = 5;
    char data[]="One\0Two\0Three\0Four\0Five";
    int length = sizeof(data);

    typedef struct SOME_KIND_OF_STRUCT {
        int arraycount;
        struct SOME_KIND_OF_STRUCT *next;
        char *array[];
    } SOME_KIND_OF_TYPE;

    SOME_KIND_OF_TYPE *skot;

    skot = malloc( sizeof(SOME_KIND_OF_TYPE)
                  + count * sizeof(char*)
                  + length );
    skot->arraycount = count;
    char *text = (char*)( skot->array+skot->arraycount );
    memcpy( text, data, length );
    int ndx;
    for ( ndx=0; ndx<skot->arraycount; ++ndx) {
        skot->array[ndx] = text;
        text += strlen(text)+1;
    }
 
    for ( ndx=0; ndx<skot->arraycount; ++ndx)
        printf( " array[%d]=%s\n", ndx, skot->array[ndx] );

    return 0;
}

johnsfine · 05-01-2013, 06:35 PM

Quote:

Originally Posted by millgates

Bad.

The usual way is

I don't disagree with that. But the Op's request is a method I used to use myself quite a lot. It is more efficient than the "usual way".

I know that efficiency on that scale usually doesn't matter. So I understand your objection to strange coding that just achieves a little extra efficiency. But if you only try efficient coding when you really need it, you won't have the right experience to get it right when you really need it.

Quote:

You can probably do it the way you attempted with some kind of very ugly hacks,

Slightly ugly, not very ugly.

Quote:

(and you would also have to set each pointer in the struct to point somewhere)

That is the detail that was getting too difficult to talk about in English, causing my decision to answer (post #5) in C rather than in English. (I thought post 4 showed the OP did not understand what I had said in post 2).

Quote:

I would not recommend this way unless you have a very good reason to do so and you know exactly what you're doing.

It isn't actually that hard. I think it is a technique worth knowing.

Mol_Bolom · 05-01-2013, 07:21 PM

Quote:

Originally Posted by johnsfine

I think this is what you are trying to do (the parts in red are a test harness around the part you want).

I don't disagree with that. But the Op's request is a method I used to use myself quite a lot.

That's it, and the last paragraph helped with one area of my blockage. It always bugs me wondering if I should do this or that.

Quote:

Originally Posted by johnsfine

That is the detail that was getting too difficult to talk about in English, causing my decision to answer (post #5) in C rather than in English. (I thought post 4 showed the OP did not understand what I had said in post 2).

Sorry about that. The older I get the more difficult it seems to be able to understand and explain things better, and I try very hard not to ask any questions. Just had a hard time getting my mind thinking on this, and just had to ask.

<Edit 2> This was explained as incorrect below by ntubski. Tested it, and it is incorrect.
Oh! By the way,

Code:

    // Shouldn't this
    char *text = (char*)( skot->array+skot->arraycount );

    // be this?
    char *text = (char*)( skot->array+skot->arraycount*sizeof(char*));
    // Because skot->arraycount would be only count bytes long?

</Edit 2>

<Edit> And the finished product with some modifications looks like this...(Including one modification learned from ntubski. Don't know about offsetof, have read through the man page, but getting late and will read more on it later).

Code:

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

typedef struct STRINGLIST_S
{
        int arrc;                   // Array count.
        struct STRINGLIST_S *next;
        struct STRINGLIST_S *prev;
        char *arrv[1];               // Array begin.
} STRINGLIST_T;


STRINGLIST_T *new_strlist(STRINGLIST_T *strlist, int listc, char *list[], int len)
{
        STRINGLIST_T *newlist = malloc(sizeof(STRINGLIST_T) +
                                       sizeof(char *)*listc +
                                       len);

        newlist->next = (strlist == NULL) ? NULL : strlist->next;
        newlist->prev = strlist;

        if (strlist != NULL) {
                if (strlist->next != NULL) strlist->next->prev = newlist;
                strlist->next = newlist;
        }
        newlist->arrc = listc;
        char *text = (char *)(newlist->arrv + newlist->arrc*sizeof(char *));
        memcpy(text, list[0], len);

        int x;
        for(x = 0;x<listc;x++)
                newlist->arrv[x] = (char *)(text+(list[x] - list[0]));

        return newlist;
}
                            


int main(void)
{
        STRINGLIST_T *strlist = NULL;
        char *alist[5] = { "This", "That", "Those", "Whatever you say", "I didn't say that" };   // 51
        char *blist[8] = { "T'was", "brillig", "in the", "slithy","toves","did gile","and gimble","in the wabe."};  //67
        // Note: The above two string arrays will never be as the above.  The string arrays will be built
        //       from a line of text read from a file. So the above code is wrong.

        strlist = new_strlist(strlist, 5, alist, 51);
        strlist = new_strlist(strlist, 8, blist, 67);

        free(strlist->prev);
        free(strlist);

        return 0;
}

ntubski · 05-01-2013, 10:53 PM

Quote:

Originally Posted by johnsfine

I always forget which compilers accept
char *array[];
vs. which accept
char *array[0];

gcc gives a warning about the second in -pedantic -std=c99 mode, and about both in -std=c89 mode. I've used

Code:

    typedef struct SOME_KIND_OF_STRUCT {
        int arraycount;
        struct SOME_KIND_OF_STRUCT *next;
        char *array[1];
    } SOME_KIND_OF_TYPE;

    SOME_KIND_OF_TYPE *skot;

    skot = malloc( offsetof(SOME_KIND_OF_TYPE, array)
                  + count * sizeof(char*)
                  + length );

Quote:

Originally Posted by Mol_Bolom

Oh! By the way,

Code:

    // Shouldn't this
    char *text = (char*)( skot->array+skot->arraycount );

    // be this?
    char *text = (char*)( skot->array+skot->arraycount*sizeof(char*));
    // Because skot->arraycount would be only count bytes long?

No, when you add an integer to an address, the type is taken into account and the compiler multiplies by sizeof(Type) for you. That is how array indexing works.

Code:

array[i] === *(array + i) 
// also
[i]array === *(i + array)

johnsfine · 05-02-2013, 05:41 AM

Quote:

Originally Posted by Mol_Bolom

Code:

        memcpy(text, list[0], len);
...
        char *alist[5] = { "This", "That", "Those", "Whatever you say", "I didn't say that" };   // 51

The compiler might store "This", "That", etc. sequentially in memory. But it is not required to, and a real compiler often won't.

Both your memcpy, and the loop you have following it assume the strings are stored sequentially. Those will go very wrong if the strings are not stored that way.

Look at my example (post #5), for how to define a collection of sequentially stored strings. That approach lets you correctly memcpy the whole set at once and it lets the compiler count up the total (vs. your example where you manually counted the total). As you can see in my code, that requires code using strlen to compute the result pointers.

If you prefer input as an array of char*, then don't assume the text is all stored contiguously (so don't try to copy it all in one memcpy). Instead, you can use a loop using strlen with either strcpy or memcpy, to compute each result pointer and copy each string. If the input is in that form, I would also suggest a preview loop to compute the total text size. Manually counting it is lame.

Mol_Bolom · 05-02-2013, 08:36 AM

Quote:

Originally Posted by johnsfine

The compiler might store "This", "That", etc. sequentially in memory. But it is not required to, and a real compiler often won't.

Both your memcpy, and the loop you have following it assume the strings are stored sequentially. Those will go very wrong if the strings are not stored that way.

Look at my example (post #5), for how to define a collection of sequentially stored strings. That approach lets you correctly memcpy the whole set at once and it lets the compiler count up the total (vs. your example where you manually counted the total). As you can see in my code, that requires code using strlen to compute the result pointers.

If you prefer input as an array of char*, then don't assume the text is all stored contiguously (so don't try to copy it all in one memcpy). Instead, you can use a loop using strlen with either strcpy or memcpy, to compute each result pointer and copy each string. If the input is in that form, I would also suggest a preview loop to compute the total text size. Manually counting it is lame.

Woops, I had suspected as much, but it was getting late and it worked at the moment and totally forgot to mention that I was never going to use it like that. Instead I will write a function which will read a line of text from a file into a char string[MAXARRAYCOUNT] (total length will be found once this is filled), and convert all the strings stored in string to a char *array[MAXARRAYCOUNT]. In theory it should work from that.

Something like as follows. (untested).

Code:

FILE *fh;
char string[MAXSTRINGLENGTH];
char *array[MAXARRAYCOUNT];
int arrc = 0;
int strlength;
STRINGLIST_T *strlist;

strlength = freadline(fh, &string[0], MAXSTRINGLENGTH); // A program specific function
                                                        // for reading a line of text.
                                                        // Never larger than MAXSTRINGLENGTH.

int x;

array[0] = &string[0];

for(x=1;x<strlength;x++) {
   if (string[x] == 0x20) {            // Every space turns into a null terminating character.
       string[x] = 0;
       arrc++;
       array[arrc] = &string[x+1];
   } else {
   if (string[x] == 0xA) {             // newline turns into a null terminating character.
       string[x] == 0;
       arrc++;
       break;                          // Exit when the string is found.
   }
   }
}

strlist = new_strlist(strlist, arrc, array[0], strlength);  // I have yet to memorize how to pass
                                                            // array, but will pick that up through
                                                            // examining and testing as I go along.

Note: I haven't exactly decided if I want to go this route as of the moment. I had been using a lot of linked lists for each array[x] for all x > 0, and doubly linked lists for all array[0]. However, I want to see how this may function verses what I had been doing. Just to get a different view on it.

<Edit>
Alright, here is the final working example (on my machine with my version of gcc at this moment in time, anyway) minus any ambiguity, hopefully.

Code:

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

#define MAXARRAYLENGTH 30

typedef struct STRINGLIST_S
{
     int arrc;
     struct STRINGLIST_S *next;
     struct STRINGLIST_S *prev;
     char *arrv[1];
} STRINGLIST_T;

STRINGLIST_T *new_strlist(STRINGLIST_T *strlist, int listc, char *list[], int len)
{
        STRINGLIST_T *newlist = malloc(sizeof(STRINGLIST_T) +
                                       sizeof(char *)*listc +
                                       len);

        newlist->next = (strlist == NULL) ? NULL : strlist->next;
        newlist->prev = strlist;

        if (strlist != NULL) {
                if (strlist->next != NULL) strlist->next->prev = newlist;
                strlist->next = newlist;
        }
        newlist->arrc = listc;
        char *text = (char *)(newlist->arrv + newlist->arrc); //*sizeof(char *));
        memcpy(text, list[0], len);

        int x;
        for(x = 0;x<listc;x++)
                newlist->arrv[x] = text+(list[x] - list[0]);

        return newlist;
}
                            
STRINGLIST_T *new_strtostrlist(STRINGLIST_T *strlist, char *string, int length)
{
    int arrc= 0;
    char *array[MAXARRAYLENGTH];
   
    array[0] = string;
    arrc++;
    while ( (array[arrc] = index(array[arrc-1], 0x20)) != NULL)
    {
            *(array[arrc]) = 0;
            array[arrc]++;
            arrc++;
    }
    return new_strlist(strlist, arrc, array, length);
}

int main(void)
{
        STRINGLIST_T *strlist = NULL;
        char alist[256] = "This that those these them huh";     // 31
        char blist[256] = "T'was brillig in the slithy toves did gile and gimble in the wabe."; // 67;

        strlist = new_strtostrlist(strlist, &alist[0], strlen(alist));
        strlist = new_strtostrlist(strlist, &blist[0], strlen(blist));

        free(strlist->prev);
        free(strlist);

        return 0;
}