On Fri, Mar 22, 2013 at 03:24:45PM +0000, Philip Herron wrote:
> ---
> cmd.c | 179
> ++++++++++++++++++++++++++++++++------------------------------
> server.c | 3 ++
> tmux.c | 11 ++++
> tmux.h | 8 ++-
> 4 files changed, 112 insertions(+), 89 deletions(-)
Some comments inline below but it would also be useful to know why you
need this? I'd be reluctant to apply it unless something else that will
go into the codebase will actually use it.
> diff --git a/cmd.c b/cmd.c
> index c8e9702..8a47e33 100644
> --- a/cmd.c
> +++ b/cmd.c
> @@ -27,93 +27,96 @@
>
> #include "tmux.h"
>
> -const struct cmd_entry *cmd_table[] = {
> - &cmd_attach_session_entry,
> - &cmd_bind_key_entry,
> - &cmd_break_pane_entry,
> - &cmd_capture_pane_entry,
> - &cmd_choose_buffer_entry,
> - &cmd_choose_client_entry,
> - &cmd_choose_list_entry,
> - &cmd_choose_session_entry,
> - &cmd_choose_tree_entry,
> - &cmd_choose_window_entry,
> - &cmd_clear_history_entry,
> - &cmd_clock_mode_entry,
> - &cmd_command_prompt_entry,
> - &cmd_confirm_before_entry,
> - &cmd_copy_mode_entry,
> - &cmd_delete_buffer_entry,
> - &cmd_detach_client_entry,
> - &cmd_display_message_entry,
> - &cmd_display_panes_entry,
> - &cmd_find_window_entry,
> - &cmd_has_session_entry,
> - &cmd_if_shell_entry,
> - &cmd_join_pane_entry,
> - &cmd_kill_pane_entry,
> - &cmd_kill_server_entry,
> - &cmd_kill_session_entry,
> - &cmd_kill_window_entry,
> - &cmd_last_pane_entry,
> - &cmd_last_window_entry,
> - &cmd_link_window_entry,
> - &cmd_list_buffers_entry,
> - &cmd_list_clients_entry,
> - &cmd_list_commands_entry,
> - &cmd_list_keys_entry,
> - &cmd_list_panes_entry,
> - &cmd_list_sessions_entry,
> - &cmd_list_windows_entry,
> - &cmd_load_buffer_entry,
> - &cmd_lock_client_entry,
> - &cmd_lock_server_entry,
> - &cmd_lock_session_entry,
> - &cmd_move_pane_entry,
> - &cmd_move_window_entry,
> - &cmd_new_session_entry,
> - &cmd_new_window_entry,
> - &cmd_next_layout_entry,
> - &cmd_next_window_entry,
> - &cmd_paste_buffer_entry,
> - &cmd_pipe_pane_entry,
> - &cmd_previous_layout_entry,
> - &cmd_previous_window_entry,
> - &cmd_refresh_client_entry,
> - &cmd_rename_session_entry,
> - &cmd_rename_window_entry,
> - &cmd_resize_pane_entry,
> - &cmd_respawn_pane_entry,
> - &cmd_respawn_window_entry,
> - &cmd_rotate_window_entry,
> - &cmd_run_shell_entry,
> - &cmd_save_buffer_entry,
> - &cmd_select_layout_entry,
> - &cmd_select_pane_entry,
> - &cmd_select_window_entry,
> - &cmd_send_keys_entry,
> - &cmd_send_prefix_entry,
> - &cmd_server_info_entry,
> - &cmd_set_buffer_entry,
> - &cmd_set_environment_entry,
> - &cmd_set_option_entry,
> - &cmd_set_window_option_entry,
> - &cmd_show_buffer_entry,
> - &cmd_show_environment_entry,
> - &cmd_show_messages_entry,
> - &cmd_show_options_entry,
> - &cmd_show_window_options_entry,
> - &cmd_source_file_entry,
> - &cmd_split_window_entry,
> - &cmd_start_server_entry,
> - &cmd_suspend_client_entry,
> - &cmd_swap_pane_entry,
> - &cmd_swap_window_entry,
> - &cmd_switch_client_entry,
> - &cmd_unbind_key_entry,
> - &cmd_unlink_window_entry,
> - &cmd_wait_for_entry,
> - NULL
> +size_t cmd_table_len = 0, cmd_table_size = 0;
> +struct cmd_entry ** cmd_table = NULL;
Globals have static storage duration so are automatically initialized to
0/NULL, also please put them one declaration on each line (I know we
don't do that for locals but we do for globals).
> +
> +const struct cmd_entry *cmd_table_builtin[] = {
> + (struct cmd_entry *)&cmd_attach_session_entry,
Why do you need all these casts?
> + (struct cmd_entry *)&cmd_bind_key_entry,
> + (struct cmd_entry *)&cmd_break_pane_entry,
> + (struct cmd_entry *)&cmd_capture_pane_entry,
> + (struct cmd_entry *)&cmd_choose_buffer_entry,
> + (struct cmd_entry *)&cmd_choose_client_entry,
> + (struct cmd_entry *)&cmd_choose_list_entry,
> + (struct cmd_entry *)&cmd_choose_session_entry,
> + (struct cmd_entry *)&cmd_choose_tree_entry,
> + (struct cmd_entry *)&cmd_choose_window_entry,
> + (struct cmd_entry *)&cmd_clear_history_entry,
> + (struct cmd_entry *)&cmd_clock_mode_entry,
> + (struct cmd_entry *)&cmd_command_prompt_entry,
> + (struct cmd_entry *)&cmd_confirm_before_entry,
> + (struct cmd_entry *)&cmd_copy_mode_entry,
> + (struct cmd_entry *)&cmd_delete_buffer_entry,
> + (struct cmd_entry *)&cmd_detach_client_entry,
> + (struct cmd_entry *)&cmd_display_message_entry,
> + (struct cmd_entry *)&cmd_display_panes_entry,
> + (struct cmd_entry *)&cmd_find_window_entry,
> + (struct cmd_entry *)&cmd_has_session_entry,
> + (struct cmd_entry *)&cmd_if_shell_entry,
> + (struct cmd_entry *)&cmd_join_pane_entry,
> + (struct cmd_entry *)&cmd_kill_pane_entry,
> + (struct cmd_entry *)&cmd_kill_server_entry,
> + (struct cmd_entry *)&cmd_kill_session_entry,
> + (struct cmd_entry *)&cmd_kill_window_entry,
> + (struct cmd_entry *)&cmd_last_pane_entry,
> + (struct cmd_entry *)&cmd_last_window_entry,
> + (struct cmd_entry *)&cmd_link_window_entry,
> + (struct cmd_entry *)&cmd_list_buffers_entry,
> + (struct cmd_entry *)&cmd_list_clients_entry,
> + (struct cmd_entry *)&cmd_list_commands_entry,
> + (struct cmd_entry *)&cmd_list_keys_entry,
> + (struct cmd_entry *)&cmd_list_panes_entry,
> + (struct cmd_entry *)&cmd_list_sessions_entry,
> + (struct cmd_entry *)&cmd_list_windows_entry,
> + (struct cmd_entry *)&cmd_load_buffer_entry,
> + (struct cmd_entry *)&cmd_lock_client_entry,
> + (struct cmd_entry *)&cmd_lock_server_entry,
> + (struct cmd_entry *)&cmd_lock_session_entry,
> + (struct cmd_entry *)&cmd_move_pane_entry,
> + (struct cmd_entry *)&cmd_move_window_entry,
> + (struct cmd_entry *)&cmd_new_session_entry,
> + (struct cmd_entry *)&cmd_new_window_entry,
> + (struct cmd_entry *)&cmd_next_layout_entry,
> + (struct cmd_entry *)&cmd_next_window_entry,
> + (struct cmd_entry *)&cmd_paste_buffer_entry,
> + (struct cmd_entry *)&cmd_pipe_pane_entry,
> + (struct cmd_entry *)&cmd_previous_layout_entry,
> + (struct cmd_entry *)&cmd_previous_window_entry,
> + (struct cmd_entry *)&cmd_refresh_client_entry,
> + (struct cmd_entry *)&cmd_rename_session_entry,
> + (struct cmd_entry *)&cmd_rename_window_entry,
> + (struct cmd_entry *)&cmd_resize_pane_entry,
> + (struct cmd_entry *)&cmd_respawn_pane_entry,
> + (struct cmd_entry *)&cmd_respawn_window_entry,
> + (struct cmd_entry *)&cmd_rotate_window_entry,
> + (struct cmd_entry *)&cmd_run_shell_entry,
> + (struct cmd_entry *)&cmd_save_buffer_entry,
> + (struct cmd_entry *)&cmd_select_layout_entry,
> + (struct cmd_entry *)&cmd_select_pane_entry,
> + (struct cmd_entry *)&cmd_select_window_entry,
> + (struct cmd_entry *)&cmd_send_keys_entry,
> + (struct cmd_entry *)&cmd_send_prefix_entry,
> + (struct cmd_entry *)&cmd_server_info_entry,
> + (struct cmd_entry *)&cmd_set_buffer_entry,
> + (struct cmd_entry *)&cmd_set_environment_entry,
> + (struct cmd_entry *)&cmd_set_option_entry,
> + (struct cmd_entry *)&cmd_set_window_option_entry,
> + (struct cmd_entry *)&cmd_show_buffer_entry,
> + (struct cmd_entry *)&cmd_show_environment_entry,
> + (struct cmd_entry *)&cmd_show_messages_entry,
> + (struct cmd_entry *)&cmd_show_options_entry,
> + (struct cmd_entry *)&cmd_show_window_options_entry,
> + (struct cmd_entry *)&cmd_source_file_entry,
> + (struct cmd_entry *)&cmd_split_window_entry,
> + (struct cmd_entry *)&cmd_start_server_entry,
> + (struct cmd_entry *)&cmd_suspend_client_entry,
> + (struct cmd_entry *)&cmd_swap_pane_entry,
> + (struct cmd_entry *)&cmd_swap_window_entry,
> + (struct cmd_entry *)&cmd_switch_client_entry,
> + (struct cmd_entry *)&cmd_unbind_key_entry,
> + (struct cmd_entry *)&cmd_unlink_window_entry,
> + (struct cmd_entry *)&cmd_wait_for_entry,
> + NULL
> };
>
> int cmd_session_better(struct session *, struct session *, int);
> @@ -209,7 +212,7 @@ cmd_free_argv(int argc, char **argv)
> struct cmd *
> cmd_parse(int argc, char **argv, const char *file, u_int line, char **cause)
> {
> - const struct cmd_entry **entryp, *entry;
> + struct cmd_entry **entryp, *entry;
> struct cmd *cmd;
> struct args *args;
> char s[BUFSIZ];
> diff --git a/server.c b/server.c
> index 4bfa918..0b66936 100644
> --- a/server.c
> +++ b/server.c
> @@ -196,6 +196,9 @@ server_start(int lockfd, char *lockfile)
>
> set_signals(server_signal_callback);
> server_loop();
> +
> + free (cmd_table);
> + cmd_table = NULL;
There is no need to free before exit().
> exit(0);
> }
>
> diff --git a/tmux.c b/tmux.c
> index 8ea91eb..1a7d249 100644
> --- a/tmux.c
> +++ b/tmux.c
> @@ -401,6 +401,17 @@ main(int argc, char **argv)
> setproctitle("%s (%s)", __progname, socket_path);
> #endif
>
> + cmd_table_size = threshold_alloc (BUILTIN_COMMAND_LENGTH);
> + cmd_table_len = BUILTIN_COMMAND_LENGTH;
> + cmd_table = (struct cmd_entry **)
> + calloc (cmd_table_size, sizeof (struct cmd_entry *));
> +
> + struct cmd_entry ** ptr = cmd_table;
> + struct cmd_entry ** entry = cmd_table_builtin;
Declarations can't go after statements or you will break the build with
GCC 2 :-).
> + for (entry = cmd_table_builtin; *entry != NULL; entry++)
> + *ptr++ = *entry;
> + *ptr = NULL;
> +
> /* Pass control to the client. */
> ev_base = osdep_event_init();
> exit(client_main(argc, argv, flags));
> diff --git a/tmux.h b/tmux.h
> index 834ac6e..f0b15a1 100644
> --- a/tmux.h
> +++ b/tmux.h
> @@ -1750,7 +1750,13 @@ struct winlink *cmd_find_pane(struct cmd_q *, const
> char *, struct session **,
> struct window_pane **);
> char *cmd_template_replace(const char *, const char *, int);
> const char *cmd_get_default_path(struct cmd_q *, const char *);
> -extern const struct cmd_entry *cmd_table[];
> +
> +#define threshold_alloc(x) (((x)+16)*3/2)
I wouldn't bother with this define. And why not just something simple
like start at the same size as the standard table and double each time?
> +#define BUILTIN_COMMAND_LENGTH 89 // 88 commands + 1 for sentinal
> +extern size_t cmd_table_len, cmd_table_size;
> +extern struct cmd_entry **cmd_table;
> +extern const struct cmd_entry *cmd_table_builtin[];
Yuck. I think I would put all these in cmd.c and have a cmd_table_init
function.
Then you can use nitems() to get the table length and don't need any
externs.
> +
> extern const struct cmd_entry cmd_attach_session_entry;
> extern const struct cmd_entry cmd_bind_key_entry;
> extern const struct cmd_entry cmd_break_pane_entry;
> --
> 1.7.10.4
>
>
> ------------------------------------------------------------------------------
> Everyone hates slow websites. So do we.
> Make your web apps faster with AppDynamics
> Download AppDynamics Lite for free today:
> http://p.sf.net/sfu/appdyn_d2d_mar
> _______________________________________________
> tmux-users mailing list
> tmux-users@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/tmux-users
------------------------------------------------------------------------------
Everyone hates slow websites. So do we.
Make your web apps faster with AppDynamics
Download AppDynamics Lite for free today:
http://p.sf.net/sfu/appdyn_d2d_mar
_______________________________________________
tmux-users mailing list
tmux-users@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/tmux-users
