Merge pull request #2486 from pqarmitage/updates

Fix some warnings identified by a static analyzer

keepalived/bfd/bfd_daemon.c

@@ -85,9 +85,9 @@ stop_bfd(int status)
 	pidfile_rm(&bfd_pidfile);
 
 	/* Clean data */
-	free_global_data(global_data);
+	free_global_data(&global_data);
 	bfd_dispatcher_release(bfd_data);
-	free_bfd_data(bfd_data);
+	free_bfd_data(&bfd_data);
 	free_bfd_buffer();
 	thread_destroy_master(master);
 	free_parent_mallocs_exit();
@@ -159,8 +159,9 @@ start_bfd(__attribute__((unused)) data_t *prev_global_data)
 		init_global_data(global_data, prev_global_data, true);
 
 	/* Update process name if necessary */
-	if ((!reload && global_data->bfd_process_name) ||
-	    (reload &&
+	if ((!prev_global_data &&		// startup
+	    global_data->bfd_process_name) ||
+	    (prev_global_data &&		// reload
 	     (!global_data->bfd_process_name != !prev_global_data->bfd_process_name ||
 	      (global_data->bfd_process_name && strcmp(global_data->bfd_process_name, prev_global_data->bfd_process_name)))))
 		set_process_name(global_data->bfd_process_name);
@@ -287,8 +288,8 @@ reload_bfd_thread(__attribute__((unused)) thread_ref_t thread)
 	signal_set(SIGCHLD, thread_child_handler, master);
 	start_bfd(old_global_data);
 
-	free_bfd_data(old_bfd_data);
-	free_global_data(old_global_data);
+	free_bfd_data(&old_bfd_data);
+	free_global_data(&old_global_data);
 
 #ifndef _ONE_PROCESS_DEBUG_
 	save_config(true, "bfd", dump_bfd_data_global);

keepalived/bfd/bfd_data.c

@@ -252,12 +252,16 @@ alloc_bfd_data(void)
 }
 
 void
-free_bfd_data(bfd_data_t *data)
+free_bfd_data(bfd_data_t **datap)
 {
+	bfd_data_t *data = *datap;
+
 	assert(data);
 
 	free_bfd_list(&data->bfd);
 	FREE(data);
+
+	*datap = NULL;
 }
 
 void

keepalived/check/check_daemon.c

@@ -181,9 +181,9 @@ checker_terminate_phase2(void)
 
 	/* Clean data */
 	if (global_data)
-		free_global_data(global_data);
+		free_global_data(&global_data);
 	if (check_data)
-		free_check_data(check_data);
+		free_check_data(&check_data);
 	free_parent_mallocs_exit();
 
 	/*
@@ -326,8 +326,9 @@ start_check(data_t *prev_global_data)
 		init_global_data(global_data, prev_global_data, true);
 
 	/* Update process name if necessary */
-	if ((!reload && global_data->lvs_process_name) ||
-	    (reload &&
+	if ((!prev_global_data &&		// startup
+	     global_data->lvs_process_name) ||
+	    (prev_global_data &&		// reload
 	     (!global_data->lvs_process_name != !prev_global_data->lvs_process_name ||
 	      (global_data->lvs_process_name && strcmp(global_data->lvs_process_name, prev_global_data->lvs_process_name)))))
 		set_process_name(global_data->lvs_process_name);
@@ -518,8 +519,8 @@ reload_check_thread(__attribute__((unused)) thread_ref_t thread)
 	start_check(old_global_data);
 
 	/* free backup data */
-	free_check_data(old_check_data);
-	free_global_data(old_global_data);
+	free_check_data(&old_check_data);
+	free_global_data(&old_global_data);
 
 #ifndef _ONE_PROCESS_DEBUG_
 	save_config(true, "check", dump_data_check);

keepalived/check/check_data.c

@@ -924,15 +924,19 @@ alloc_check_data(void)
 }
 
 void
-free_check_data(check_data_t *data)
+free_check_data(check_data_t **datap)
 {
+	check_data_t *data = *datap;
+
 	free_vs_list(&data->vs);
 	free_vsg_list(&data->vs_group);
 	free_track_file_list(&data->track_files);
 #ifdef _WITH_BFD_
 	free_checker_bfd_list(&data->track_bfds);
 #endif
 	FREE(data);
+
+	*datap = NULL;
 }
 
 static void

keepalived/core/global_data.c

@@ -424,8 +424,10 @@ init_global_data(data_t * data, data_t *prev_global_data, bool copy_unchangeable
 }
 
 void
-free_global_data(data_t * data)
+free_global_data(data_t **datap)
 {
+	data_t *data = *datap;
+
 	if (!data)
 		return;
 
@@ -502,6 +504,8 @@ free_global_data(data_t * data)
 #endif
 	FREE_CONST_PTR(data->config_directory);
 	FREE(data);
+
+	*datap = NULL;
 }
 
 FILE * __attribute__((malloc))

keepalived/core/main.c

@@ -831,7 +831,7 @@ static bool reload_config(void)
 
 	if (unsupported_change) {
 		/* We cannot reload the configuration, so continue with the old config */
-		free_global_data (global_data);
+		free_global_data(&global_data);
 		global_data = old_global_data;
 	}
 	else {
@@ -840,7 +840,7 @@ static bool reload_config(void)
 		    (global_data->process_name && strcmp(global_data->process_name, old_global_data->process_name)))
 			set_process_name(global_data->process_name);
 
-		free_global_data (old_global_data);
+		free_global_data(&old_global_data);
 	}
 
 	/* There is no point checking the script security of the
@@ -2910,7 +2910,7 @@ keepalived_main(int argc, char **argv)
 
 	free_parent_mallocs_startup(false);
 	free_parent_mallocs_exit();
-	free_global_data(global_data);
+	free_global_data(&global_data);
 
 	closelog();
 

keepalived/include/bfd_data.h

@@ -51,7 +51,7 @@ extern void dump_bfd_data(FILE *, const bfd_data_t *);
 extern void dump_bfd_data_global(FILE *);
 #endif
 extern void bfd_print_data(void);
-extern void free_bfd_data(bfd_data_t *);
+extern void free_bfd_data(bfd_data_t **);
 extern void bfd_complete_init(void);
 extern void alloc_bfd_buffer(void);
 extern void free_bfd_buffer(void);

keepalived/include/check_data.h

@@ -319,7 +319,7 @@ extern void alloc_ssvr(const char *, const char *);
 extern void free_checker_bfd(checker_tracked_bfd_t *);
 #endif
 extern check_data_t *alloc_check_data(void);
-extern void free_check_data(check_data_t *);
+extern void free_check_data(check_data_t **);
 extern void dump_data_check(FILE *);
 extern const char *format_vs (const virtual_server_t *);
 extern const char *format_vsge (const virtual_server_group_entry_t *);

keepalived/include/global_data.h

@@ -305,7 +305,7 @@ extern const char * format_email_addr(const char *);
 extern void alloc_email(const char *);
 extern data_t *alloc_global_data(void);
 extern void init_global_data(data_t *, data_t *, bool);
-extern void free_global_data(data_t *);
+extern void free_global_data(data_t **);
 extern FILE *open_dump_file(const char *) __attribute__((malloc));
 extern void dump_global_data(FILE *, data_t *);
 

keepalived/include/vrrp_data.h

@@ -101,7 +101,7 @@ extern void alloc_vrrp_vrule(const vector_t *);
 extern void alloc_vrrp_buffer(size_t);
 extern void free_vrrp_buffer(void);
 extern vrrp_data_t *alloc_vrrp_data(void);
-extern void free_vrrp_data(vrrp_data_t *);
+extern void free_vrrp_data(vrrp_data_t **);
 extern void free_sync_group(vrrp_sgroup_t *);
 extern void free_sock_list(list_head_t *);
 extern void dump_sock_list(FILE *, const list_head_t *);

keepalived/vrrp/vrrp_daemon.c

@@ -324,8 +324,8 @@ vrrp_terminate_phase2(int exit_status)
 	if (global_data->disable_local_igmp)
 		reset_disable_local_igmp();
 
-	free_global_data(global_data);
-	free_vrrp_data(vrrp_data);
+	free_global_data(&global_data);
+	free_vrrp_data(&vrrp_data);
 	free_vrrp_buffer();
 	free_interface_queue();
 	free_parent_mallocs_exit();
@@ -526,8 +526,9 @@ start_vrrp(data_t *prev_global_data)
 	init_data(conf_file, vrrp_init_keywords, false);
 
 	/* Update process name if necessary */
-	if ((!reload && global_data->vrrp_process_name) ||
-	    (reload &&
+	if ((!prev_global_data && 		// startup
+	     global_data->vrrp_process_name) ||
+	    (prev_global_data &&		// reload
 	     (!global_data->vrrp_process_name != !prev_global_data->vrrp_process_name ||
 	      (global_data->vrrp_process_name && strcmp(global_data->vrrp_process_name, prev_global_data->vrrp_process_name)))))
 		set_process_name(global_data->vrrp_process_name);
@@ -893,10 +894,8 @@ reload_vrrp_thread(__attribute__((unused)) thread_ref_t thread)
 #endif
 
 	/* free backup data */
-	free_vrrp_data(old_vrrp_data);
-	old_vrrp_data = NULL;
-	free_global_data(old_global_data);
-	old_global_data = NULL;
+	free_vrrp_data(&old_vrrp_data);
+	free_global_data(&old_global_data);
 
 	free_old_interface_queue();
 

keepalived/vrrp/vrrp_data.c

@@ -1260,8 +1260,10 @@ alloc_vrrp_data(void)
 }
 
 void
-free_vrrp_data(vrrp_data_t * data)
+free_vrrp_data(vrrp_data_t ** datap)
 {
+	vrrp_data_t *data = *datap;
+
 	free_ipaddress_list(&data->static_addresses);
 	free_iproute_list(&data->static_routes);
 	free_iprule_list(&data->static_rules);
@@ -1277,6 +1279,8 @@ free_vrrp_data(vrrp_data_t * data)
 #endif
 	free_vrrp_list(&data->vrrp);
 	FREE(data);
+
+	*datap = NULL;
 }
 
 static void

keepalived/vrrp/vrrp_dbus.c

@@ -136,8 +136,8 @@ static GMainLoop *loop;
 
 /* Data passing between main vrrp thread and dbus thread */
 dbus_queue_ent_t *ent_ptr;
-static int dbus_in_pipe[2] = {-1, -1};
-static int dbus_out_pipe[2] = {-1, -1};
+static int dbus_in_pipe[2] = { -1 };	// [0] == -1 indicates pipe is closed
+static int dbus_out_pipe[2] = { -1 };	// Ditto
 static sem_t thread_end;
 
 /* The only characters that are valid in a dbus path are A-Z, a-z, 0-9, _ */
@@ -942,14 +942,12 @@ dbus_start_error(dbus_files_t *files)
 		close(dbus_in_pipe[0]);
 		close(dbus_in_pipe[1]);
 		dbus_in_pipe[0] = -1;
-		dbus_in_pipe[1] = -1;
 	}
 
 	if (dbus_out_pipe[0] != -1) {
 		close(dbus_out_pipe[0]);
 		close(dbus_out_pipe[1]);
 		dbus_out_pipe[0] = -1;
-		dbus_out_pipe[1] = -1;
 	}
 
 	return false;
@@ -1074,11 +1072,9 @@ dbus_stop(void)
 	close(dbus_in_pipe[0]);
 	close(dbus_in_pipe[1]);
 	dbus_in_pipe[0] = -1;
-	dbus_in_pipe[0] = -1;
 	close(dbus_out_pipe[0]);
 	close(dbus_out_pipe[1]);
 	dbus_out_pipe[0] = -1;
-	dbus_out_pipe[0] = -1;
 }
 
 #ifdef THREAD_DUMP