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 connman@connman.net http://lists.connman.net/listinfo/connman