Hi Jukka,
> >> +struct connman_config_item **connman_config_get_configs(void)
> >> +{
> >> + GHashTableIter iter_file, iter_config;
> >> + gpointer value, key;
> >> + struct connman_config_item **configs = NULL;
> >> + int i, count = 0;
> >> +
> >> + g_hash_table_iter_init(&iter_file, config_table);
> >> + while (g_hash_table_iter_next(&iter_file, &key, &value) == TRUE) {
> >> + struct connman_config *config_file = value;
> >> +
> >> + g_hash_table_iter_init(&iter_config,
> >> + config_file->service_table);
> >> + while (g_hash_table_iter_next(&iter_config, &key,
> >> + &value) == TRUE) {
> >> + struct connman_config_service *config = value;
> >> + configs = g_try_realloc(configs, (count + 1) *
> >> + sizeof(struct connman_config_item *));
> >> + if (configs == NULL)
> >> + return NULL;
> >
> > Why are we doing this? We know the size of hash table. So allocate this
> > ahead of time,
>
> We need to traverse two hashes so it complicates the issue. I changed
> the code but IMHO the code looks now horrible.
not sure if it can get more bad. Lets try and then we see.
> >
> >> +
> >> + configs[count] =
> >> + g_try_new0(struct connman_config_item, 1);
> >> + if (configs[count] == NULL)
> >> + goto cleanup;
> >
> > And why this? Is not something simple like
> >
> > g_try_new0(struct connman_config_item, count)
> >
> > sufficient?
>
> We have already allocated the array where the pointers should be, so it
> makes it much simpler to allocate the individual structs one by one.
I do not see how this can be better. If you know how much to allocate,
then just allocate the struct space as well. The structs are all same
size, so I don't see any benefit of all these memory allocations.
Regards
Marcel
_______________________________________________
connman mailing list
[email protected]
http://lists.connman.net/listinfo/connman
