Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix some warnings identified by a static analyzer #2486

Merged
merged 3 commits into from
Oct 23, 2024
Merged

Fix some warnings identified by a static analyzer #2486

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
061b264
all: change checking process name at reload to include not NULL checks
pqarmitage Oct 23, 2024
43889d1
vrrp: Only use dbus_{in,out}_pipe[0] to indicate pipe is closed
pqarmitage Oct 23, 2024
3f67cc6
all: clear pointers to old data structures freed after reload
pqarmitage Oct 23, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 7 additions & 6 deletions keepalived/bfd/bfd_daemon.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand Down
6 changes: 5 additions & 1 deletion keepalived/bfd/bfd_data.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
13 changes: 7 additions & 6 deletions keepalived/check/check_daemon.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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();

/*
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand Down
6 changes: 5 additions & 1 deletion keepalived/check/check_data.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
6 changes: 5 additions & 1 deletion keepalived/core/global_data.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -502,6 +504,8 @@ free_global_data(data_t * data)
#endif
FREE_CONST_PTR(data->config_directory);
FREE(data);

*datap = NULL;
}

FILE * __attribute__((malloc))
Expand Down
6 changes: 3 additions & 3 deletions keepalived/core/main.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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
Expand Down Expand Up @@ -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();

Expand Down
2 changes: 1 addition & 1 deletion keepalived/include/bfd_data.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
2 changes: 1 addition & 1 deletion keepalived/include/check_data.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 *);
Expand Down
2 changes: 1 addition & 1 deletion keepalived/include/global_data.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 *);

Expand Down
2 changes: 1 addition & 1 deletion keepalived/include/vrrp_data.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 *);
Expand Down
15 changes: 7 additions & 8 deletions keepalived/vrrp/vrrp_daemon.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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();

Expand Down
6 changes: 5 additions & 1 deletion keepalived/vrrp/vrrp_data.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -1277,6 +1279,8 @@ free_vrrp_data(vrrp_data_t * data)
#endif
free_vrrp_list(&data->vrrp);
FREE(data);

*datap = NULL;
}

static void
Expand Down
8 changes: 2 additions & 6 deletions keepalived/vrrp/vrrp_dbus.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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, _ */
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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
Expand Down
Loading