diff --git a/common/file.c b/common/file.c index 3a66bba3..a0ef888a 100644 --- a/common/file.c +++ b/common/file.c @@ -100,7 +100,7 @@ l_file_read_sections(int fd, int max_file_size, struct list *names) { if (line_lookup_for_section_name(text, FILE_MAX_LINE_BYTES) != 0) { - list_add_item(names, (tbus)g_strdup(text)); + list_add_strdup(names, text); } } } @@ -286,7 +286,7 @@ l_file_read_section(int fd, int max_file_size, const char *section, if (g_strlen(text) > 0) { file_split_name_value(text, name, value); - list_add_item(names, (tbus)g_strdup(name)); + list_add_strdup(names, name); if (value[0] == '$') { @@ -294,16 +294,16 @@ l_file_read_section(int fd, int max_file_size, const char *section, if (lvalue != 0) { - list_add_item(values, (tbus)g_strdup(lvalue)); + list_add_strdup(values, lvalue); } else { - list_add_item(values, (tbus)g_strdup("")); + list_add_strdup(values, ""); } } else { - list_add_item(values, (tbus)g_strdup(value)); + list_add_strdup(values, value); } } } diff --git a/common/list.c b/common/list.c index 79375e03..b5039869 100644 --- a/common/list.c +++ b/common/list.c @@ -23,6 +23,7 @@ #endif #include +#include #include "arch.h" #include "os_calls.h" @@ -232,6 +233,65 @@ list_insert_item(struct list *self, int index, tbus item) return 1; } + +/******************************************************************************/ +int +list_add_strdup(struct list *self, const char *str) +{ + int rv; + char *dup; + + if (str == NULL) + { + rv = list_add_item(self, (tintptr)str); + } + else if ((dup = g_strdup(str)) == NULL) + { + rv = 0; + } + else + { + rv = list_add_item(self, (tintptr)dup); + if (!rv) + { + g_free(dup); + } + } + + return rv; +} + +/******************************************************************************/ +int +list_add_strdup_multi(struct list *self, ...) +{ + va_list ap; + int entry_count = self->count; + const char *s; + int rv = 1; + + va_start(ap, self); + while ((s = va_arg(ap, const char *)) != NULL) + { + if (!list_add_strdup(self, s)) + { + rv = 0; + break; + } + } + va_end(ap); + + if (rv == 0) + { + // Remove the additional items we added + while (self->count > entry_count) + { + list_remove_item(self, self->count - 1); + } + } + + return rv; +} /******************************************************************************/ /* append one list to another using strdup for each item in the list */ /* begins copy at start_index, a zero based index on the source list */ @@ -245,23 +305,7 @@ list_append_list_strdup(struct list *self, struct list *dest, int start_index) for (index = start_index; index < self->count; index++) { const char *item = (const char *)list_get_item(self, index); - char *dup; - if (item == NULL) - { - // This shouldn't really happen, but if it does we'll - // copy the item anyway. - dup = NULL; - } - else - { - dup = g_strdup(item); - if (dup == NULL) - { - rv = 0; - break; - } - } - if (!list_add_item(dest, (tbus)dup)) + if (!list_add_strdup(dest, item)) { rv = 0; break; diff --git a/common/list.h b/common/list.h index 41b9fe76..c330fa8a 100644 --- a/common/list.h +++ b/common/list.h @@ -113,4 +113,37 @@ list_dump_items(struct list *self); struct list * split_string_into_list(const char *str, char character); +/** + * As list_add_item() but for a C string + * + * This is a convenience function for a common operation + * @param self List to append to + * @param str String to append + * + * The passed-in string is strdup'd onto the list, so if auto_free + * isn't set, memory leaks will occur. + * + * A NULL pointer will be added as a NULL entry. + * + * @result 0 if any memory allocation failure occurred. In this case + * the list is unchanged. + */ + +int +list_add_strdup(struct list *self, const char *str); + +/** + * Add multiple strings to a list + * + * This is a convenience function for a common operation + * @param self List to append to + * @param ... Strings to append. Terminate the list with a NULL. + * + * @result 0 if any memory allocation failure occurred. In this case + * the list is unchanged. + */ + +int +list_add_strdup_multi(struct list *self, ...); + #endif diff --git a/sesman/config.c b/sesman/config.c index fc477ca0..34ef69ae 100644 --- a/sesman/config.c +++ b/sesman/config.c @@ -452,8 +452,8 @@ config_read_xorg_params(int file, struct config_sesman *cs, for (i = 0; i < param_n->count; i++) { - list_add_item(cs->xorg_params, - (long) g_strdup((char *) list_get_item(param_v, i))); + list_add_strdup(cs->xorg_params, + (const char *) list_get_item(param_v, i)); } return 0; @@ -485,7 +485,8 @@ config_read_vnc_params(int file, struct config_sesman *cs, struct list *param_n, for (i = 0; i < param_n->count; i++) { - list_add_item(cs->vnc_params, (long)g_strdup((char *)list_get_item(param_v, i))); + list_add_strdup(cs->vnc_params, + (const char *)list_get_item(param_v, i)); } return 0; @@ -510,10 +511,10 @@ config_read_session_variables(int file, struct config_sesman *cs, for (i = 0; i < param_n->count; i++) { - list_add_item(cs->env_names, - (tintptr) g_strdup((char *) list_get_item(param_n, i))); - list_add_item(cs->env_values, - (tintptr) g_strdup((char *) list_get_item(param_v, i))); + list_add_strdup(cs->env_names, + (const char *) list_get_item(param_n, i)); + list_add_strdup(cs->env_values, + (const char *) list_get_item(param_v, i)); } return 0; diff --git a/sesman/session.c b/sesman/session.c index 77af95cb..123a29fd 100644 --- a/sesman/session.c +++ b/sesman/session.c @@ -360,7 +360,7 @@ session_start_chansrv(int uid, int display) g_snprintf(exe_path, sizeof(exe_path), "%s/xrdp-chansrv", XRDP_SBIN_PATH); - list_add_item(chansrv_params, (intptr_t) g_strdup(exe_path)); + list_add_strdup(chansrv_params, exe_path); list_add_item(chansrv_params, 0); /* mandatory */ env_set_user(uid, 0, display, @@ -759,10 +759,10 @@ session_start(struct auth_info *auth_info, xserver = g_strdup((const char *)list_get_item(g_cfg->xorg_params, 0)); /* these are the must have parameters */ - list_add_item(xserver_params, (tintptr) g_strdup(xserver)); - list_add_item(xserver_params, (tintptr) g_strdup(screen)); - list_add_item(xserver_params, (tintptr) g_strdup("-auth")); - list_add_item(xserver_params, (tintptr) g_strdup(authfile)); + list_add_strdup_multi(xserver_params, + xserver, screen, + "-auth", authfile, + NULL); /* additional parameters from sesman.ini file */ list_append_list_strdup(g_cfg->xorg_params, xserver_params, 1); @@ -791,17 +791,13 @@ session_start(struct auth_info *auth_info, xserver = g_strdup((const char *)list_get_item(g_cfg->vnc_params, 0)); /* these are the must have parameters */ - list_add_item(xserver_params, (tintptr)g_strdup(xserver)); - list_add_item(xserver_params, (tintptr)g_strdup(screen)); - list_add_item(xserver_params, (tintptr)g_strdup("-auth")); - list_add_item(xserver_params, (tintptr)g_strdup(authfile)); - list_add_item(xserver_params, (tintptr)g_strdup("-geometry")); - list_add_item(xserver_params, (tintptr)g_strdup(geometry)); - list_add_item(xserver_params, (tintptr)g_strdup("-depth")); - list_add_item(xserver_params, (tintptr)g_strdup(depth)); - list_add_item(xserver_params, (tintptr)g_strdup("-rfbauth")); - list_add_item(xserver_params, (tintptr)g_strdup(passwd_file)); - + list_add_strdup_multi(xserver_params, + xserver, screen, + "-auth", authfile, + "-geometry", geometry, + "-depth", depth, + "-rfbauth", passwd_file, + NULL); g_free(passwd_file); /* additional parameters from sesman.ini file */ diff --git a/tests/common/test_list_calls.c b/tests/common/test_list_calls.c index a4a48fd7..75705e08 100644 --- a/tests/common/test_list_calls.c +++ b/tests/common/test_list_calls.c @@ -65,7 +65,15 @@ START_TEST(test_list__simple_auto_free) { char strval[64]; g_snprintf(strval, sizeof(strval), "%d", i); - list_add_item(lst, (tintptr)g_strdup(strval)); + // Odds, use list_add_item/strdup, evens use list_add_strdup + if ((i % 2) != 0) + { + list_add_item(lst, (tintptr)g_strdup(strval)); + } + else + { + list_add_strdup(lst, strval); + } } list_remove_item(lst, 0); @@ -116,6 +124,29 @@ START_TEST(test_list__simple_append_list) } END_TEST +START_TEST(test_list__simple_strdup_multi) +{ + int i; + struct list *lst = list_create(); + lst->auto_free = 1; + + list_add_strdup_multi(lst, + "0", "1", "2", "3", "4", "5", + "6", "7", "8", "9", "10", "11", + NULL); + + ck_assert_int_eq(lst->count, 12); + + for (i = 0 ; i < lst->count; ++i) + { + int val = g_atoi((const char *)list_get_item(lst, i)); + ck_assert_int_eq(val, i); + } + + list_delete(lst); +} +END_TEST + int split_string_append_fragment(const char **start, const char *end, struct list *list); @@ -172,6 +203,7 @@ make_suite_test_list(void) tcase_add_test(tc_simple, test_list__simple); tcase_add_test(tc_simple, test_list__simple_auto_free); tcase_add_test(tc_simple, test_list__simple_append_list); + tcase_add_test(tc_simple, test_list__simple_strdup_multi); tcase_add_test(tc_simple, test_list__append_fragment); tcase_add_test(tc_simple, test_list__split_string_into_list); diff --git a/xrdp/xrdp_login_wnd.c b/xrdp/xrdp_login_wnd.c index a97467ab..8b6062f4 100644 --- a/xrdp/xrdp_login_wnd.c +++ b/xrdp/xrdp_login_wnd.c @@ -652,11 +652,11 @@ xrdp_wm_login_fill_in_combo(struct xrdp_wm *self, struct xrdp_bitmap *b) g_strncpy(name, r, 255); } - list_add_item(mod_data->names, (long)g_strdup(q)); - list_add_item(mod_data->values, (long)g_strdup(r)); + list_add_strdup(mod_data->names, q); + list_add_strdup(mod_data->values, r); } - list_add_item(b->string_list, (long)g_strdup(name)); + list_add_strdup(b->string_list, name); list_add_item(b->data_list, (long)mod_data); } } diff --git a/xrdp/xrdp_mm.c b/xrdp/xrdp_mm.c index c6dee2a8..9644c8ec 100644 --- a/xrdp/xrdp_mm.c +++ b/xrdp/xrdp_mm.c @@ -486,8 +486,8 @@ xrdp_mm_setup_mod2(struct xrdp_mm *self) the module should use the last one */ if (g_strlen(text) > 0) { - list_add_item(self->login_names, (long)g_strdup("port")); - list_add_item(self->login_values, (long)g_strdup(text)); + list_add_strdup(self->login_names, "port"); + list_add_strdup(self->login_values, text); } /* always set these */ diff --git a/xrdp/xrdp_wm.c b/xrdp/xrdp_wm.c index f4915247..dc546220 100644 --- a/xrdp/xrdp_wm.c +++ b/xrdp/xrdp_wm.c @@ -758,8 +758,8 @@ xrdp_wm_init(struct xrdp_wm *self) } } - list_add_item(self->mm->login_names, (long)g_strdup(q)); - list_add_item(self->mm->login_values, (long)g_strdup(r)); + list_add_strdup(self->mm->login_names, q); + list_add_strdup(self->mm->login_values, r); } /*