Re: [PATCH 6/7] CLI: make static message strings const

Subject: Re: [PATCH 6/7] CLI: make static message strings const

Date: Thu, 13 May 2021 20:27:44 +0300

To: David Bremner, notmuch@notmuchmail.org

Cc: David Bremner

From: Tomi Ollila


On Thu, May 13 2021, David Bremner wrote:

> This is both a bit clearer and avoids the possibility of modification.
> ---
>  notmuch-insert.c | 2 +-
>  notmuch-tag.c    | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/notmuch-insert.c b/notmuch-insert.c
> index 00c00468..e3d87e4a 100644
> --- a/notmuch-insert.c
> +++ b/notmuch-insert.c
> @@ -34,7 +34,7 @@ static volatile sig_atomic_t interrupted;
>  static void
>  handle_sigint (unused (int sig))
>  {
> -    static char msg[] = "Stopping...         \n";
> +    static const char msg[] = "Stopping...         \n";

If compiler is smart enough, it can even save copy from read-only
area of the binary to read-write runtime memory area.

If not that smart it does the copy.

In my projects I've been using the following macro:
#define WriteCS(fd, str) write((fd), (str), sizeof(str) - 1)
but I don't know if that behaved any better (if we cared)...

But, in addition to these two identical copies of handle_sigint()
also notmuch-new.c and notmuch-reindex.c defines anoter 2 identical
copies of handle_sigint()...

I did not see that those two were also modified in this series...

Other that these, the series looked ok...

Tomi

>  
>      /* This write is "opportunistic", so it's okay to ignore the
>       * result.  It is not required for correctness, and if it does
> diff --git a/notmuch-tag.c b/notmuch-tag.c
> index 667a75d6..de428c8e 100644
> --- a/notmuch-tag.c
> +++ b/notmuch-tag.c
> @@ -27,7 +27,7 @@ static volatile sig_atomic_t interrupted;
>  static void
>  handle_sigint (unused (int sig))
>  {
> -    static char msg[] = "Stopping...         \n";
> +    static const char msg[] = "Stopping...         \n";
>  
>      /* This write is "opportunistic", so it's okay to ignore the
>       * result.  It is not required for correctness, and if it does
> -- 
> 2.30.2
> _______________________________________________
> notmuch mailing list -- notmuch@notmuchmail.org
> To unsubscribe send an email to notmuch-leave@notmuchmail.org
_______________________________________________
notmuch mailing list -- notmuch@notmuchmail.org
To unsubscribe send an email to notmuch-leave@notmuchmail.org

Thread: