Re: allocating memory for array of pointers to char
From: Aaron Walker (ka0ttic_at_cfl.rr.com)
Date: 03/08/04
- Next message: Clark Cox: "Re: gcc warning not understood"
- Previous message: Olivier MERIGON: "Re: gcc warning not understood"
- In reply to: Richard Bos: "Re: allocating memory for array of pointers to char"
- Next in thread: xarax: "Re: allocating memory for array of pointers to char"
- Reply: xarax: "Re: allocating memory for array of pointers to char"
- Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]
Date: Mon, 08 Mar 2004 15:31:49 GMT
On Mon, 08 Mar 2004 12:33:18 +0000, Richard Bos wrote:
> Irrelevant code, OK, but please do leave in all declarations. For
> checking up on allocation and so on, the types of your objects are
> important.
First of all, thanks for the response.
>
>> option->nvalue = nvalue;
>> option->value = calloc(nvalue, sizeof(char *));
>
> How is option declared? If option->value is a pointer (it seems to be a
> char **), note that calloc() allocates all-bits-zero memory, and all-
> bits-zero is not guaranteed to be a null pointer. You might as well use
> malloc() and possibly save some cycles.
option is a pointer to a structure, which is encapsulated in a void
pointer inside a linked-list element. Once option is filled in, its
appended to the linked list.
>
>> for(j = 0; j < nvalue; j++)
>> {
>> register int a;
>
> It is rarely useful to specify register yourself; your compiler is
> likely to be better at optimisation at that level than you, these days.
>
>> char value[sizeof(buf)];
>>
>> for(a = 0; !isspace(buf[i]); a++, i++)
>> value[a] = buf[i];
>> value[a] = '\0';
>>
>> option->value = ec_strdup(value);
>
>> FYI, ec_strdup() just calls malloc() and strcpy().
>
> How? That's important, you know.
char *
ec_strdup(const char *str)
{
char *cp;
cp = malloc(strlen(str) + 1);
strcpy(cp, str);
return cp;
}
>
>> I tried the above w/o the calloc() first, but it segfaults.
>
> If ec_strdup() calls malloc() the right way, there should not be a
> problem with the segfault. In fact, if it does, the code you have here
> probably leaks memory. However, you've snipped almost all significant
> code - to whit, that which actually declares the pointers and allocates
> the memory - so there's no way I can be definite.
I'll try to be more thorough this time (at the bottom).
>
>> Doing it this way works, although I get
>> "free(): invalid pointer 0x804b838!" when trying to free option->value[0]
>> (for the same exact "value" each time) at runtime,
>
> Well, if you want to free(option->value[0]), you need to point
> option->value[0] at allocated memory. The way you have it above,
> option->value probably points at allocated memory, and option->value[0]
> probably does not.
even though I do: option->value[j] = ec_strdup() ?
I cannot loop 0 through option->nvalue (number of values in option->value) and free()?
>
>> Is this improper? If so, what would I need to do?
>
> The first thing you need to do is sit down, not behind a keyboard but
> behind a piece of paper, and design your data structures, thoroughly.
I've spent quite a few hours on a notepad, actually. I've just never
messed with pointers to pointers that much, especially dynamically
allocated ones.. This is also the first time I've used linked lists, so
maybe the number of pointers and all the indirection is confusing me.
>
>> I can't just calloc() since the length of each char pointer is variable, right?
>
> No, the length of a char pointer is sizeof (char *). But possibly each
> char * points at a different size block of memory.
>
> Richard
Ok, I'll try this again ;)
<code>
typedef struct listelem_t {
void *data;
struct listelem_t *next;
} ListElem;
typedef struct {
ListElem *head;
ListElem *tail;
int size;
void (*cleanup)(void *data);
} List;
typedef struct {
char *key;
char **value;
int nvalue;
} ConfigOption;
static List *list;
static void cleanup_options(void *);
void
read_config(char *config_file)
{
...
list = ec_malloc(sizeof(List)); /* ec_malloc() is just an error
checking wrapper */
create_list(list, &cleanup_options);
...
while(fgets(...)) {
int i, j;
int nvalue;
char inside_word;
char key[sizeof(buf)];
ConfigOption *option;
...
option = ec_malloc(sizeof(ConfigOption));
...
option->key = ec_strdup(key);
...
option->nvalue = nvalue;
option->value = ec_calloc(nvalue, sizeof(char *));
for( /* loop through nvalue... */ ) {
char value[sizeof(buf)];
...
option->value[j] = ec_strdup(value);
}
insert_elem(list, list->tail, (void *)option);
...
}
...
}
/* this gets set to the cleanup member of struct List
* and gets called for each list element in cleanup_list()
*/
static void
cleanup_options(void *data)
{
int i;
ConfigOption *option = (ConfigOption *)data;
free(option->key);
/* I get the free() message on the first interation
* of this loop for the only multi-value element in
* linked-list
*/
for(i = 0; i < option->nvalue; i++)
free(option->nvalue[i]);
free(option->nvalue);
free(option);
}
</code>
After typing that last function in (the code is currently on my laptop, so
I can't cut & paste), I had an idea of what I might be doing wrong (as far
as the free'ing goes). You were saying about option->value[i] not
pointing to allocated memory. Is it because I need to dereference it
above?
Hope this is enough for you to understand what I am trying to do. Once
again, thanks very much for your help.
Aaron
- Next message: Clark Cox: "Re: gcc warning not understood"
- Previous message: Olivier MERIGON: "Re: gcc warning not understood"
- In reply to: Richard Bos: "Re: allocating memory for array of pointers to char"
- Next in thread: xarax: "Re: allocating memory for array of pointers to char"
- Reply: xarax: "Re: allocating memory for array of pointers to char"
- Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]
Relevant Pages
|
