Mailing List Archive

[PATCH] constify static arrays of strings
By marking these static & the array itself as const, the data is moved
into the read-only section of memory. Otherwise a writable array is
created which takes up more space or code execution time. So this will
both shrink & slightly speed up the code for free.
---
auth-rhosts.c | 2 +-
dns.c | 2 +-
kex.c | 2 +-
nchan.c | 8 ++++++--
regress/misc/fuzz-harness/kex_fuzz.cc | 2 +-
sftp-server.c | 2 +-
ssh-keygen.c | 2 +-
sshconnect2.c | 2 +-
utf8.c | 4 +++-
9 files changed, 16 insertions(+), 10 deletions(-)

diff --git a/auth-rhosts.c b/auth-rhosts.c
index 0bc4d424c781..7022d6a6f0f5 100644
--- a/auth-rhosts.c
+++ b/auth-rhosts.c
@@ -191,7 +191,7 @@ auth_rhosts2(struct passwd *pw, const char *client_user, const char *hostname,
{
char buf[1024];
struct stat st;
- static const char *rhosts_files[] = {".shosts", ".rhosts", NULL};
+ static const char * const rhosts_files[] = {".shosts", ".rhosts", NULL};
u_int rhosts_file_index;

debug2("auth_rhosts2: clientuser %s hostname %s ipaddr %s",
diff --git a/dns.c b/dns.c
index 1cfc38e7cc43..286b4021960a 100644
--- a/dns.c
+++ b/dns.c
@@ -43,7 +43,7 @@
#include "log.h"
#include "digest.h"

-static const char *errset_text[] = {
+static const char * const errset_text[] = {
"success", /* 0 ERRSET_SUCCESS */
"out of memory", /* 1 ERRSET_NOMEMORY */
"general failure", /* 2 ERRSET_FAIL */
diff --git a/kex.c b/kex.c
index 709a0ec63aa0..195819f1d953 100644
--- a/kex.c
+++ b/kex.c
@@ -66,7 +66,7 @@
static int kex_choose_conf(struct ssh *);
static int kex_input_newkeys(int, u_int32_t, struct ssh *);

-static const char *proposal_names[PROPOSAL_MAX] = {
+static const char * const proposal_names[PROPOSAL_MAX] = {
"KEX algorithms",
"host key algorithms",
"ciphers ctos",
diff --git a/nchan.c b/nchan.c
index 7ef3a350b79a..72a1a8358aa2 100644
--- a/nchan.c
+++ b/nchan.c
@@ -82,8 +82,12 @@ static void chan_shutdown_write(struct ssh *, Channel *);
static void chan_shutdown_read(struct ssh *, Channel *);
static void chan_shutdown_extended_read(struct ssh *, Channel *);

-static const char *ostates[] = { "open", "drain", "wait_ieof", "closed" };
-static const char *istates[] = { "open", "drain", "wait_oclose", "closed" };
+static const char * const ostates[] = {
+ "open", "drain", "wait_ieof", "closed",
+};
+static const char * const istates[] = {
+ "open", "drain", "wait_oclose", "closed",
+};

static void
chan_set_istate(Channel *c, u_int next)
diff --git a/regress/misc/fuzz-harness/kex_fuzz.cc b/regress/misc/fuzz-harness/kex_fuzz.cc
index 4740a7cb04c1..d1198fa6bc66 100644
--- a/regress/misc/fuzz-harness/kex_fuzz.cc
+++ b/regress/misc/fuzz-harness/kex_fuzz.cc
@@ -327,7 +327,7 @@ int main(void)
static struct shared_state *st;
struct test_state *ts;
const int keytypes[] = { KEY_RSA, KEY_DSA, KEY_ECDSA, KEY_ED25519, -1 };
- const char *kextypes[] = {
+ static const char * const kextypes[] = {
"sntrup761x25519-sha512@openssh.com",
"curve25519-sha256@libssh.org",
"ecdh-sha2-nistp256",
diff --git a/sftp-server.c b/sftp-server.c
index 18d194911257..efef57cadb0d 100644
--- a/sftp-server.c
+++ b/sftp-server.c
@@ -517,7 +517,7 @@ send_msg(struct sshbuf *m)
static const char *
status_to_message(u_int32_t status)
{
- const char *status_messages[] = {
+ static const char * const status_messages[] = {
"Success", /* SSH_FX_OK */
"End of file", /* SSH_FX_EOF */
"No such file", /* SSH_FX_NO_SUCH_FILE */
diff --git a/ssh-keygen.c b/ssh-keygen.c
index 4b40768d517f..76c6c726c17f 100644
--- a/ssh-keygen.c
+++ b/ssh-keygen.c
@@ -2459,7 +2459,7 @@ load_sign_key(const char *keypath, const struct sshkey *pubkey)
{
size_t i, slen, plen = strlen(keypath);
char *privpath = xstrdup(keypath);
- const char *suffixes[] = { "-cert.pub", ".pub", NULL };
+ static const char * const suffixes[] = { "-cert.pub", ".pub", NULL };
struct sshkey *ret = NULL, *privkey = NULL;
int r;

diff --git a/sshconnect2.c b/sshconnect2.c
index fea50fab61d3..808ff8d38e91 100644
--- a/sshconnect2.c
+++ b/sshconnect2.c
@@ -1318,7 +1318,7 @@ identity_sign(struct identity *id, u_char **sigp, size_t *lenp,
static int
id_filename_matches(Identity *id, Identity *private_id)
{
- const char *suffixes[] = { ".pub", "-cert.pub", NULL };
+ static const char * const suffixes[] = { ".pub", "-cert.pub", NULL };
size_t len = strlen(id->filename), plen = strlen(private_id->filename);
size_t i, slen;

diff --git a/utf8.c b/utf8.c
index 7f63b25aeefc..5cafe21e611f 100644
--- a/utf8.c
+++ b/utf8.c
@@ -325,7 +325,9 @@ mprintf(const char *fmt, ...)
void
msetlocale(void)
{
- const char *vars[] = { "LC_ALL", "LC_CTYPE", "LANG", NULL };
+ static const char * const vars[] = {
+ "LC_ALL", "LC_CTYPE", "LANG", NULL,
+ };
char *cp;
int i;

--
2.33.0

_______________________________________________
openssh-unix-dev mailing list
openssh-unix-dev@mindrot.org
https://lists.mindrot.org/mailman/listinfo/openssh-unix-dev
Re: [PATCH] constify static arrays of strings [ In reply to ]
On Thu, 30 Sep 2021, Mike Frysinger wrote:

> By marking these static & the array itself as const, the data is moved

> - static const char *rhosts_files[] = {".shosts", ".rhosts", NULL};
> + static const char * const rhosts_files[] = {".shosts", ".rhosts", NULL};

Is this really true nowadays? I thought with PIC/PIE, these are
relocations and therefore neither constant nor shared/shareable,
at least for some architectures (i386?).

But it’s nevertheless a good idea, of course.

bye,
//mirabilos
--
Sometimes they [people] care too much: pretty printers [and syntax highligh-
ting, d.A.] mechanically produce pretty output that accentuates irrelevant
detail in the program, which is as sensible as putting all the prepositions
in English text in bold font. -- Rob Pike in "Notes on Programming in C"
_______________________________________________
openssh-unix-dev mailing list
openssh-unix-dev@mindrot.org
https://lists.mindrot.org/mailman/listinfo/openssh-unix-dev
Re: [PATCH] constify static arrays of strings [ In reply to ]
On 30 Sep 2021 20:10, Thorsten Glaser wrote:
> On Thu, 30 Sep 2021, Mike Frysinger wrote:
> > By marking these static & the array itself as const, the data is moved
>
> > - static const char *rhosts_files[] = {".shosts", ".rhosts", NULL};
> > + static const char * const rhosts_files[] = {".shosts", ".rhosts", NULL};
>
> Is this really true nowadays? I thought with PIC/PIE, these are
> relocations and therefore neither constant nor shared/shareable,
> at least for some architectures (i386?).

i assume `size` doesn't lie to me ;). on my current Linux build:

### sftp-server.o
function old new delta
static.status_messages - 80 +80
send_status 600 627 +27
status_to_message 278 - -278
------------------------------------------------------------------------------
(add/remove: 1/1 grow/shrink: 1/0 up/down: 107/-278) Total: -171 bytes

### sshconnect2.o
function old new delta
static.suffixes - 24 +24
sign_and_send_pubkey 3838 3758 -80
------------------------------------------------------------------------------
(add/remove: 1/0 grow/shrink: 0/1 up/down: 24/-80) Total: -56 bytes

### ssh-keygen.o
function old new delta
static.suffixes - 24 +24
main 14558 14522 -36
------------------------------------------------------------------------------
(add/remove: 1/0 grow/shrink: 0/1 up/down: 24/-36) Total: -12 bytes

### utf8.o
function old new delta
static.vars - 32 +32
msetlocale 401 324 -77
------------------------------------------------------------------------------
(add/remove: 1/0 grow/shrink: 0/1 up/down: 32/-77) Total: -45 bytes

you can see it shifting between writable sections to read-only sections too.

Gentoo gcc has PIE enabled by default.
$ gcc -v
Using built-in specs.
COLLECT_GCC=/usr/bin/gcc
COLLECT_LTO_WRAPPER=/usr/libexec/gcc/x86_64-pc-linux-gnu/11.2.0/lto-wrapper
Target: x86_64-pc-linux-gnu
Configured with: /var/tmp/portage/sys-devel/gcc-11.2.0/work/gcc-11.2.0/configure --host=x86_64-pc-linux-gnu --build=x86_64-pc-linux-gnu --prefix=/usr --bindir=/usr/x86_64-pc-linux-gnu/gcc-bin/11.2.0 --includedir=/usr/lib/gcc/x86_64-pc-linux-gnu/11.2.0/include --datadir=/usr/share/gcc-data/x86_64-pc-linux-gnu/11.2.0 --mandir=/usr/share/gcc-data/x86_64-pc-linux-gnu/11.2.0/man --infodir=/usr/share/gcc-data/x86_64-pc-linux-gnu/11.2.0/info --with-gxx-include-dir=/usr/lib/gcc/x86_64-pc-linux-gnu/11.2.0/include/g++-v11 --with-python-dir=/share/gcc-data/x86_64-pc-linux-gnu/11.2.0/python --enable-languages=c,c++,go,fortran --enable-obsolete --enable-secureplt --disable-werror --with-system-zlib --enable-nls --without-included-gettext --disable-libunwind-exceptions --enable-checking=release --with-bugurl=https://bugs.gentoo.org/ --with-pkgversion='Gentoo 11.2.0 p1' --disable-esp --enable-libstdcxx-time --enable-shared --enable-threads=posix --enable-__cxa_atexit --enable-clocale=gnu --enable-multilib --with-multilib-list=mx32,m32,m64 --disable-fixed-point --with-abi=m64 --enable-targets=all --enable-libgomp --disable-libssp --disable-libada --disable-systemtap --disable-valgrind-annotations --disable-vtable-verify --disable-libvtv --without-zstd --enable-lto --with-isl --disable-isl-version-check --enable-default-pie --enable-default-ssp
Thread model: posix
Supported LTO compression algorithms: zlib
gcc version 11.2.0 (Gentoo 11.2.0 p1)
-mike
Re: [PATCH] constify static arrays of strings [ In reply to ]
ping ...
-mike

On 30 Sep 2021 13:44, Mike Frysinger wrote:
> By marking these static & the array itself as const, the data is moved
> into the read-only section of memory. Otherwise a writable array is
> created which takes up more space or code execution time. So this will
> both shrink & slightly speed up the code for free.
> ---
> auth-rhosts.c | 2 +-
> dns.c | 2 +-
> kex.c | 2 +-
> nchan.c | 8 ++++++--
> regress/misc/fuzz-harness/kex_fuzz.cc | 2 +-
> sftp-server.c | 2 +-
> ssh-keygen.c | 2 +-
> sshconnect2.c | 2 +-
> utf8.c | 4 +++-
> 9 files changed, 16 insertions(+), 10 deletions(-)
>
> diff --git a/auth-rhosts.c b/auth-rhosts.c
> index 0bc4d424c781..7022d6a6f0f5 100644
> --- a/auth-rhosts.c
> +++ b/auth-rhosts.c
> @@ -191,7 +191,7 @@ auth_rhosts2(struct passwd *pw, const char *client_user, const char *hostname,
> {
> char buf[1024];
> struct stat st;
> - static const char *rhosts_files[] = {".shosts", ".rhosts", NULL};
> + static const char * const rhosts_files[] = {".shosts", ".rhosts", NULL};
> u_int rhosts_file_index;
>
> debug2("auth_rhosts2: clientuser %s hostname %s ipaddr %s",
> diff --git a/dns.c b/dns.c
> index 1cfc38e7cc43..286b4021960a 100644
> --- a/dns.c
> +++ b/dns.c
> @@ -43,7 +43,7 @@
> #include "log.h"
> #include "digest.h"
>
> -static const char *errset_text[] = {
> +static const char * const errset_text[] = {
> "success", /* 0 ERRSET_SUCCESS */
> "out of memory", /* 1 ERRSET_NOMEMORY */
> "general failure", /* 2 ERRSET_FAIL */
> diff --git a/kex.c b/kex.c
> index 709a0ec63aa0..195819f1d953 100644
> --- a/kex.c
> +++ b/kex.c
> @@ -66,7 +66,7 @@
> static int kex_choose_conf(struct ssh *);
> static int kex_input_newkeys(int, u_int32_t, struct ssh *);
>
> -static const char *proposal_names[PROPOSAL_MAX] = {
> +static const char * const proposal_names[PROPOSAL_MAX] = {
> "KEX algorithms",
> "host key algorithms",
> "ciphers ctos",
> diff --git a/nchan.c b/nchan.c
> index 7ef3a350b79a..72a1a8358aa2 100644
> --- a/nchan.c
> +++ b/nchan.c
> @@ -82,8 +82,12 @@ static void chan_shutdown_write(struct ssh *, Channel *);
> static void chan_shutdown_read(struct ssh *, Channel *);
> static void chan_shutdown_extended_read(struct ssh *, Channel *);
>
> -static const char *ostates[] = { "open", "drain", "wait_ieof", "closed" };
> -static const char *istates[] = { "open", "drain", "wait_oclose", "closed" };
> +static const char * const ostates[] = {
> + "open", "drain", "wait_ieof", "closed",
> +};
> +static const char * const istates[] = {
> + "open", "drain", "wait_oclose", "closed",
> +};
>
> static void
> chan_set_istate(Channel *c, u_int next)
> diff --git a/regress/misc/fuzz-harness/kex_fuzz.cc b/regress/misc/fuzz-harness/kex_fuzz.cc
> index 4740a7cb04c1..d1198fa6bc66 100644
> --- a/regress/misc/fuzz-harness/kex_fuzz.cc
> +++ b/regress/misc/fuzz-harness/kex_fuzz.cc
> @@ -327,7 +327,7 @@ int main(void)
> static struct shared_state *st;
> struct test_state *ts;
> const int keytypes[] = { KEY_RSA, KEY_DSA, KEY_ECDSA, KEY_ED25519, -1 };
> - const char *kextypes[] = {
> + static const char * const kextypes[] = {
> "sntrup761x25519-sha512@openssh.com",
> "curve25519-sha256@libssh.org",
> "ecdh-sha2-nistp256",
> diff --git a/sftp-server.c b/sftp-server.c
> index 18d194911257..efef57cadb0d 100644
> --- a/sftp-server.c
> +++ b/sftp-server.c
> @@ -517,7 +517,7 @@ send_msg(struct sshbuf *m)
> static const char *
> status_to_message(u_int32_t status)
> {
> - const char *status_messages[] = {
> + static const char * const status_messages[] = {
> "Success", /* SSH_FX_OK */
> "End of file", /* SSH_FX_EOF */
> "No such file", /* SSH_FX_NO_SUCH_FILE */
> diff --git a/ssh-keygen.c b/ssh-keygen.c
> index 4b40768d517f..76c6c726c17f 100644
> --- a/ssh-keygen.c
> +++ b/ssh-keygen.c
> @@ -2459,7 +2459,7 @@ load_sign_key(const char *keypath, const struct sshkey *pubkey)
> {
> size_t i, slen, plen = strlen(keypath);
> char *privpath = xstrdup(keypath);
> - const char *suffixes[] = { "-cert.pub", ".pub", NULL };
> + static const char * const suffixes[] = { "-cert.pub", ".pub", NULL };
> struct sshkey *ret = NULL, *privkey = NULL;
> int r;
>
> diff --git a/sshconnect2.c b/sshconnect2.c
> index fea50fab61d3..808ff8d38e91 100644
> --- a/sshconnect2.c
> +++ b/sshconnect2.c
> @@ -1318,7 +1318,7 @@ identity_sign(struct identity *id, u_char **sigp, size_t *lenp,
> static int
> id_filename_matches(Identity *id, Identity *private_id)
> {
> - const char *suffixes[] = { ".pub", "-cert.pub", NULL };
> + static const char * const suffixes[] = { ".pub", "-cert.pub", NULL };
> size_t len = strlen(id->filename), plen = strlen(private_id->filename);
> size_t i, slen;
>
> diff --git a/utf8.c b/utf8.c
> index 7f63b25aeefc..5cafe21e611f 100644
> --- a/utf8.c
> +++ b/utf8.c
> @@ -325,7 +325,9 @@ mprintf(const char *fmt, ...)
> void
> msetlocale(void)
> {
> - const char *vars[] = { "LC_ALL", "LC_CTYPE", "LANG", NULL };
> + static const char * const vars[] = {
> + "LC_ALL", "LC_CTYPE", "LANG", NULL,
> + };
> char *cp;
> int i;
>
> --
> 2.33.0
>
Re: [PATCH] constify static arrays of strings [ In reply to ]
applied - thanks!

On Tue, 1 Feb 2022, Mike Frysinger wrote:

> ping ...
> -mike
>
> On 30 Sep 2021 13:44, Mike Frysinger wrote:
> > By marking these static & the array itself as const, the data is moved
> > into the read-only section of memory. Otherwise a writable array is
> > created which takes up more space or code execution time. So this will
> > both shrink & slightly speed up the code for free.
> > ---
> > auth-rhosts.c | 2 +-
> > dns.c | 2 +-
> > kex.c | 2 +-
> > nchan.c | 8 ++++++--
> > regress/misc/fuzz-harness/kex_fuzz.cc | 2 +-
> > sftp-server.c | 2 +-
> > ssh-keygen.c | 2 +-
> > sshconnect2.c | 2 +-
> > utf8.c | 4 +++-
> > 9 files changed, 16 insertions(+), 10 deletions(-)
> >
> > diff --git a/auth-rhosts.c b/auth-rhosts.c
> > index 0bc4d424c781..7022d6a6f0f5 100644
> > --- a/auth-rhosts.c
> > +++ b/auth-rhosts.c
> > @@ -191,7 +191,7 @@ auth_rhosts2(struct passwd *pw, const char *client_user, const char *hostname,
> > {
> > char buf[1024];
> > struct stat st;
> > - static const char *rhosts_files[] = {".shosts", ".rhosts", NULL};
> > + static const char * const rhosts_files[] = {".shosts", ".rhosts", NULL};
> > u_int rhosts_file_index;
> >
> > debug2("auth_rhosts2: clientuser %s hostname %s ipaddr %s",
> > diff --git a/dns.c b/dns.c
> > index 1cfc38e7cc43..286b4021960a 100644
> > --- a/dns.c
> > +++ b/dns.c
> > @@ -43,7 +43,7 @@
> > #include "log.h"
> > #include "digest.h"
> >
> > -static const char *errset_text[] = {
> > +static const char * const errset_text[] = {
> > "success", /* 0 ERRSET_SUCCESS */
> > "out of memory", /* 1 ERRSET_NOMEMORY */
> > "general failure", /* 2 ERRSET_FAIL */
> > diff --git a/kex.c b/kex.c
> > index 709a0ec63aa0..195819f1d953 100644
> > --- a/kex.c
> > +++ b/kex.c
> > @@ -66,7 +66,7 @@
> > static int kex_choose_conf(struct ssh *);
> > static int kex_input_newkeys(int, u_int32_t, struct ssh *);
> >
> > -static const char *proposal_names[PROPOSAL_MAX] = {
> > +static const char * const proposal_names[PROPOSAL_MAX] = {
> > "KEX algorithms",
> > "host key algorithms",
> > "ciphers ctos",
> > diff --git a/nchan.c b/nchan.c
> > index 7ef3a350b79a..72a1a8358aa2 100644
> > --- a/nchan.c
> > +++ b/nchan.c
> > @@ -82,8 +82,12 @@ static void chan_shutdown_write(struct ssh *, Channel *);
> > static void chan_shutdown_read(struct ssh *, Channel *);
> > static void chan_shutdown_extended_read(struct ssh *, Channel *);
> >
> > -static const char *ostates[] = { "open", "drain", "wait_ieof", "closed" };
> > -static const char *istates[] = { "open", "drain", "wait_oclose", "closed" };
> > +static const char * const ostates[] = {
> > + "open", "drain", "wait_ieof", "closed",
> > +};
> > +static const char * const istates[] = {
> > + "open", "drain", "wait_oclose", "closed",
> > +};
> >
> > static void
> > chan_set_istate(Channel *c, u_int next)
> > diff --git a/regress/misc/fuzz-harness/kex_fuzz.cc b/regress/misc/fuzz-harness/kex_fuzz.cc
> > index 4740a7cb04c1..d1198fa6bc66 100644
> > --- a/regress/misc/fuzz-harness/kex_fuzz.cc
> > +++ b/regress/misc/fuzz-harness/kex_fuzz.cc
> > @@ -327,7 +327,7 @@ int main(void)
> > static struct shared_state *st;
> > struct test_state *ts;
> > const int keytypes[] = { KEY_RSA, KEY_DSA, KEY_ECDSA, KEY_ED25519, -1 };
> > - const char *kextypes[] = {
> > + static const char * const kextypes[] = {
> > "sntrup761x25519-sha512@openssh.com",
> > "curve25519-sha256@libssh.org",
> > "ecdh-sha2-nistp256",
> > diff --git a/sftp-server.c b/sftp-server.c
> > index 18d194911257..efef57cadb0d 100644
> > --- a/sftp-server.c
> > +++ b/sftp-server.c
> > @@ -517,7 +517,7 @@ send_msg(struct sshbuf *m)
> > static const char *
> > status_to_message(u_int32_t status)
> > {
> > - const char *status_messages[] = {
> > + static const char * const status_messages[] = {
> > "Success", /* SSH_FX_OK */
> > "End of file", /* SSH_FX_EOF */
> > "No such file", /* SSH_FX_NO_SUCH_FILE */
> > diff --git a/ssh-keygen.c b/ssh-keygen.c
> > index 4b40768d517f..76c6c726c17f 100644
> > --- a/ssh-keygen.c
> > +++ b/ssh-keygen.c
> > @@ -2459,7 +2459,7 @@ load_sign_key(const char *keypath, const struct sshkey *pubkey)
> > {
> > size_t i, slen, plen = strlen(keypath);
> > char *privpath = xstrdup(keypath);
> > - const char *suffixes[] = { "-cert.pub", ".pub", NULL };
> > + static const char * const suffixes[] = { "-cert.pub", ".pub", NULL };
> > struct sshkey *ret = NULL, *privkey = NULL;
> > int r;
> >
> > diff --git a/sshconnect2.c b/sshconnect2.c
> > index fea50fab61d3..808ff8d38e91 100644
> > --- a/sshconnect2.c
> > +++ b/sshconnect2.c
> > @@ -1318,7 +1318,7 @@ identity_sign(struct identity *id, u_char **sigp, size_t *lenp,
> > static int
> > id_filename_matches(Identity *id, Identity *private_id)
> > {
> > - const char *suffixes[] = { ".pub", "-cert.pub", NULL };
> > + static const char * const suffixes[] = { ".pub", "-cert.pub", NULL };
> > size_t len = strlen(id->filename), plen = strlen(private_id->filename);
> > size_t i, slen;
> >
> > diff --git a/utf8.c b/utf8.c
> > index 7f63b25aeefc..5cafe21e611f 100644
> > --- a/utf8.c
> > +++ b/utf8.c
> > @@ -325,7 +325,9 @@ mprintf(const char *fmt, ...)
> > void
> > msetlocale(void)
> > {
> > - const char *vars[] = { "LC_ALL", "LC_CTYPE", "LANG", NULL };
> > + static const char * const vars[] = {
> > + "LC_ALL", "LC_CTYPE", "LANG", NULL,
> > + };
> > char *cp;
> > int i;
> >
> > --
> > 2.33.0
> >
>
_______________________________________________
openssh-unix-dev mailing list
openssh-unix-dev@mindrot.org
https://lists.mindrot.org/mailman/listinfo/openssh-unix-dev