Re: [PATCH 1/1] Client: Added support for the history file.

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Maico,

On Thu, Apr 12, 2018 at 1:34 PM, Maico Timmerman
<maico.timmerman@xxxxxxxxx> wrote:
> Individual interactive tools can set a history file, which will be read
> on startup and written on clean-up.
> ---
>  AUTHORS            |  1 +
>  client/main.c      |  1 +
>  src/shared/shell.c | 23 +++++++++++++++++++++++
>  src/shared/shell.h |  2 ++
>  4 files changed, 27 insertions(+)
>
> diff --git a/AUTHORS b/AUTHORS
> index df9cb96ad..d2edbf749 100644
> --- a/AUTHORS
> +++ b/AUTHORS
> @@ -100,3 +100,4 @@ Bharat Panda <bharat.panda@xxxxxxxxxxx>
>  Marie Janssen <jamuraa@xxxxxxxxxxxx>
>  Jaganath Kanakkassery <jaganath.k@xxxxxxxxxxx>
>  Michał Narajowski <michal.narajowski@xxxxxxxxxxx>
> +Maico Timmerman <maico.timmerman@xxxxxxxxx>

Id leave this for a separate patch, usually we only add people to
AUTHORS when they contribute substential code to the project.

> diff --git a/client/main.c b/client/main.c
> index b96278d45..ed536a720 100644
> --- a/client/main.c
> +++ b/client/main.c
> @@ -2544,6 +2544,7 @@ int main(int argc, char *argv[])
>         bt_shell_add_submenu(&scan_menu);
>         bt_shell_add_submenu(&gatt_menu);
>         bt_shell_set_prompt(PROMPT_OFF);
> +       bt_shell_set_history_file(".bluetoothctl_history");

Id enable the history by default, the shell could figure out the
binary name from argv as well. Besides Id update the tools in a
separate patch.

>         if (agent_option)
>                 auto_register_agent = g_strdup(agent_option);
> diff --git a/src/shared/shell.c b/src/shared/shell.c
> index be2a8dfe0..39583de1e 100644
> --- a/src/shared/shell.c
> +++ b/src/shared/shell.c
> @@ -38,6 +38,7 @@
>
>  #include <readline/readline.h>
>  #include <readline/history.h>
> +#include <pwd.h>
>
>  #include "src/shared/mainloop.h"
>  #include "src/shared/timeout.h"
> @@ -69,6 +70,8 @@ static struct {
>         int timeout;
>         struct io *input;
>
> +       char *bt_shell_history;

Call it just history.

> +
>         bool saved_prompt;
>         bt_shell_prompt_input_func saved_func;
>         void *saved_user_data;
> @@ -1027,6 +1030,11 @@ void bt_shell_cleanup(void)
>         bt_shell_release_prompt("");
>         bt_shell_detach();
>
> +       if (data.bt_shell_history) {
> +               write_history(data.bt_shell_history);
> +               history_truncate_file(data.bt_shell_history, 500);
> +       }

If the tools crashes or bt_shell_cleanup is not called for whatever
reason it appears we would lose the history, perhaps it is a it would
be better to set a timer once something is written to the history and
then update the history using that, well if the history size is small
otherwise it may be too much io to keep updating it frequently.

>         if (data.envs) {
>                 queue_destroy(data.envs, env_destroy);
>                 data.envs = NULL;
> @@ -1079,6 +1087,21 @@ bool bt_shell_add_submenu(const struct bt_shell_menu *menu)
>         return true;
>  }
>
> +void bt_shell_set_history_file(const char *string)
> +{
> +       char *homedir;
> +
> +       // First check HOME env variable, otherwise read homedir from /etc/passwd.

Comments should be using /* */

> +       if ((homedir = getenv("HOME")) == NULL) {
> +               homedir = getpwuid(getuid())->pw_dir;
> +       }
> +       data.bt_shell_history = strcat(strcat(homedir, "/"), string);;

Perhaps we could write to current dir if HOME is not set, Im also not
sure writing to use home directly is a good idea. Perhaps we should
check with XDG spec:

https://standards.freedesktop.org/basedir-spec/basedir-spec-latest.html

Probably it makes sense to use XDG_CACHE_HOME for these files.

> +       // Read history file and set index of history beyond latest position.
> +       read_history(data.bt_shell_history);
> +       history_set_pos(history_length);
> +}
> +
>  void bt_shell_set_prompt(const char *string)
>  {
>         if (!data.init || data.mode)
> diff --git a/src/shared/shell.h b/src/shared/shell.h
> index 8b7cb7f30..1c4ff0e7b 100644
> --- a/src/shared/shell.h
> +++ b/src/shared/shell.h
> @@ -79,6 +79,8 @@ bool bt_shell_remove_submenu(const struct bt_shell_menu *menu);
>
>  void bt_shell_set_prompt(const char *string);
>
> +void bt_shell_set_history_file(const char *string);
> +
>  void bt_shell_printf(const char *fmt,
>                                 ...) __attribute__((format(printf, 1, 2)));
>  void bt_shell_hexdump(const unsigned char *buf, size_t len);
> --
> 2.17.0
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at  http://vger.kernel.org/majordomo-info.html



-- 
Luiz Augusto von Dentz
--
To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Bluez Devel]     [Linux Wireless Networking]     [Linux Wireless Personal Area Networking]     [Linux ATH6KL]     [Linux USB Devel]     [Linux Media Drivers]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Big List of Linux Books]

  Powered by Linux