* Extend usage of const char* @ 2024-01-01 18:10 Jörg Sommer 2024-01-01 18:10 ` [PATCH 1/6] zle.textobjects: Mark variables as const Jörg Sommer 0 siblings, 1 reply; 10+ messages in thread From: Jörg Sommer @ 2024-01-01 18:10 UTC (permalink / raw) To: zsh-workers The compiler option `-Wwrite-strings` brings up many places where a string literal (`const char*`) gets assigned to a variable whose type (`char*`) allows modifications of the string content. If the DATA segment of the ELF binary is mapped as read-only, this leads to a segmentation violation and crashes the process. But even without a crash, the compiler could do better checks and optimizations, if the variables have the appropriate type. These patches are the beginning of a series of many commits—if you like. So, please tell, if I should merge commits, reorder or rewrite them. Many of them are trivial changes of adding `const` to the variable declaration. But some of them require code rearrangements or rewrites. Hence, I plan to commit one file at a time, if possible. [PATCH 1/6] zle.textobjects: Mark variables as const [PATCH 2/6] lex: Mark variables with const init as const [PATCH 3/6] zle_vi: Mark variables with const init as const [PATCH 4/6] zle_main: mark statusline as const [PATCH 5/6] module: Mark name argument of some functions const [PATCH 6/6] zsh: mark hookdef.name as const Src/Zle/textobjects.c | 6 +++--- Src/Zle/zle_main.c | 2 +- Src/Zle/zle_vi.c | 2 +- Src/lex.c | 6 +++--- Src/module.c | 15 ++++++++------- Src/zsh.h | 2 +- 6 files changed, 17 insertions(+), 16 deletions(-) ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 1/6] zle.textobjects: Mark variables as const 2024-01-01 18:10 Extend usage of const char* Jörg Sommer @ 2024-01-01 18:10 ` Jörg Sommer 2024-01-01 18:10 ` [PATCH 2/6] lex: Mark variables with const init " Jörg Sommer ` (4 more replies) 0 siblings, 5 replies; 10+ messages in thread From: Jörg Sommer @ 2024-01-01 18:10 UTC (permalink / raw) To: zsh-workers; +Cc: Jörg Sommer Because these variables are initialized with as constant string, they should be marked as *const* to make the compiler running with `-Wwrite-strings` more happy. --- Src/Zle/textobjects.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Src/Zle/textobjects.c b/Src/Zle/textobjects.c index c93777b65..a68c5296e 100644 --- a/Src/Zle/textobjects.c +++ b/Src/Zle/textobjects.c @@ -283,9 +283,9 @@ selectargument(UNUSED(char **args)) free(linein); if (IS_THINGY(bindk, selectinshellword)) { - ZLE_CHAR_T *match = ZWS("`\'\""); - ZLE_CHAR_T *lmatch = ZWS("\'({"), *rmatch = ZWS("\')}"); - ZLE_CHAR_T *ematch = match, *found; + const ZLE_CHAR_T *match = ZWS("`\'\""); + const ZLE_CHAR_T *lmatch = ZWS("\'({"), *rmatch = ZWS("\')}"); + const ZLE_CHAR_T *ematch = match, *found; int start, end = zlecs; /* for 'in' widget, don't include initial blanks ... */ while (mark < zlecs && ZC_iblank(zleline[mark])) -- 2.43.0 ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 2/6] lex: Mark variables with const init as const 2024-01-01 18:10 ` [PATCH 1/6] zle.textobjects: Mark variables as const Jörg Sommer @ 2024-01-01 18:10 ` Jörg Sommer 2024-01-26 7:39 ` Oliver Kiddle 2024-01-01 18:10 ` [PATCH 3/6] zle_vi: " Jörg Sommer ` (3 subsequent siblings) 4 siblings, 1 reply; 10+ messages in thread From: Jörg Sommer @ 2024-01-01 18:10 UTC (permalink / raw) To: zsh-workers; +Cc: Jörg Sommer Because these variables are initialized with as constant string, they should be marked as *const* to make the compiler running with `-Wwrite-strings` more happy. --- Src/lex.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Src/lex.c b/Src/lex.c index 31b130b07..e2aa059cd 100644 --- a/Src/lex.c +++ b/Src/lex.c @@ -168,7 +168,7 @@ static struct lexbufstate lexbuf_raw; /* text of punctuation tokens */ /**/ -mod_export char *tokstrings[WHILE + 1] = { +mod_export const char *tokstrings[WHILE + 1] = { NULL, /* NULLTOK 0 */ ";", /* SEPER */ "\\n", /* NEWLIN */ @@ -410,8 +410,8 @@ void initlextabs(void) { int t0; - static char *lx1 = "\\q\n;!&|(){}[]<>"; - static char *lx2 = ";)|$[]~({}><=\\\'\"`,-!"; + static const char *lx1 = "\\q\n;!&|(){}[]<>"; + static const char *lx2 = ";)|$[]~({}><=\\\'\"`,-!"; for (t0 = 0; t0 != 256; t0++) { lexact1[t0] = LX1_OTHER; -- 2.43.0 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/6] lex: Mark variables with const init as const 2024-01-01 18:10 ` [PATCH 2/6] lex: Mark variables with const init " Jörg Sommer @ 2024-01-26 7:39 ` Oliver Kiddle 2024-01-27 6:41 ` Jörg Sommer 0 siblings, 1 reply; 10+ messages in thread From: Oliver Kiddle @ 2024-01-26 7:39 UTC (permalink / raw) To: Jörg Sommer; +Cc: zsh-workers On 1 Jan, Jörg Sommer wrote: > Because these variables are initialized with as constant string, they should > be marked as *const* to make the compiler running with `-Wwrite-strings` > more happy. Applying this patch series causes a couple of fresh warnings: zle_thingy.c: In function 'bin_zle_refresh': zle_thingy.c:420:15: warning: initialization discards 'const' qualifier from pointer target type [-Wdiscarded-qualifiers] 420 | char *s = statusline; | lex.c: In function 'exalias': lex.c:1964:20: warning: assignment discards 'const' qualifier from pointer target type [-Wdiscarded-qualifiers] 1964 | zshlextext = tokstrings[tok]; | clang describes these as "initializing 'char *' with an expression of type 'const char *' discards qualifiers [-Wincompatible-pointer-types-discards-qualifiers]". I have no objection to adding const in more places where possible. You may find we have code that is not really modifying its parameter but relies on being able to temporarily swap a nul in for a character as part of parsing. I also tend to find the old issue that you can't safely cast a char ** to const char** tends to get in the way. Oliver ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/6] lex: Mark variables with const init as const 2024-01-26 7:39 ` Oliver Kiddle @ 2024-01-27 6:41 ` Jörg Sommer 2024-01-28 0:18 ` Oliver Kiddle 0 siblings, 1 reply; 10+ messages in thread From: Jörg Sommer @ 2024-01-27 6:41 UTC (permalink / raw) To: Oliver Kiddle; +Cc: zsh-workers [-- Attachment #1: Type: text/plain, Size: 1997 bytes --] Oliver Kiddle schrieb am Fr 26. Jan, 08:39 (+0100): > On 1 Jan, Jörg Sommer wrote: > > Because these variables are initialized with as constant string, they should > > be marked as *const* to make the compiler running with `-Wwrite-strings` > > more happy. > > Applying this patch series causes a couple of fresh warnings: > > zle_thingy.c: In function 'bin_zle_refresh': > zle_thingy.c:420:15: warning: initialization discards 'const' qualifier from pointer target type [-Wdiscarded-qualifiers] > 420 | char *s = statusline; > | > > lex.c: In function 'exalias': > lex.c:1964:20: warning: assignment discards 'const' qualifier from pointer target type [-Wdiscarded-qualifiers] > 1964 | zshlextext = tokstrings[tok]; > | > > clang describes these as "initializing 'char *' with an expression of > type 'const char *' discards qualifiers > [-Wincompatible-pointer-types-discards-qualifiers]". Yes, it's a kind of chicken egg problem. Either it gets a really big patch or you have steps that introduce new warnings and solve them with later patches. I don't know what you prefer for review. > I have no objection to adding const in more places where possible. You may > find we have code that is not really modifying its parameter but relies on > being able to temporarily swap a nul in for a character as part of > parsing. Unfortunately, in the sense of *const* this is a modification. And I think I saw a function that expects to have modifiable memory, if it contains a separator character. If it not, because its not user input and hence pre-splitted, it doesn't modify the memory (and is const). Sorry, I don't remember the name after that time. My main question is how this patch set should be organized? One commit per file which reduces the number of warnings in this file to the cost of new warnings in other files? Regards Jörg -- Wenn du nur einen Hammer hast, sieht jedes Problem aus wie ein Nagel. [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 269 bytes --] ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/6] lex: Mark variables with const init as const 2024-01-27 6:41 ` Jörg Sommer @ 2024-01-28 0:18 ` Oliver Kiddle 0 siblings, 0 replies; 10+ messages in thread From: Oliver Kiddle @ 2024-01-28 0:18 UTC (permalink / raw) To: Jörg Sommer; +Cc: zsh-workers Jörg Sommer wrote: > Yes, it's a kind of chicken egg problem. Either it gets a really big patch > or you have steps that introduce new warnings and solve them with later > patches. I don't know what you prefer for review. For now, I've applied those four (of the six) patches that don't introduce new warnings. > My main question is how this patch set should be organized? One commit per > file which reduces the number of warnings in this file to the cost of new > warnings in other files? Separating patches by source file is probably not the most helpful. I'd prefer if at least each patch series doesn't add new warnings so that we're leaving the source tree in a decent state when pushing. There might be some things that initially look like they clearly should be marked const but as the const percolates through you get stuck. Perhaps start with something like statusline, resolve the knock-on effects of marking that const and at that point finalise it as a single patch. If a subset of the changes can be applied standalone, consider separating that out. But really, whatever works for you is fine. Oliver ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 3/6] zle_vi: Mark variables with const init as const 2024-01-01 18:10 ` [PATCH 1/6] zle.textobjects: Mark variables as const Jörg Sommer 2024-01-01 18:10 ` [PATCH 2/6] lex: Mark variables with const init " Jörg Sommer @ 2024-01-01 18:10 ` Jörg Sommer 2024-01-01 18:10 ` [PATCH 4/6] zle_main: mark statusline " Jörg Sommer ` (2 subsequent siblings) 4 siblings, 0 replies; 10+ messages in thread From: Jörg Sommer @ 2024-01-01 18:10 UTC (permalink / raw) To: zsh-workers; +Cc: Jörg Sommer Because these variables are initialized with as constant string, they should be marked as *const* to make the compiler running with `-Wwrite-strings` more happy. --- Src/Zle/zle_vi.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Src/Zle/zle_vi.c b/Src/Zle/zle_vi.c index 24d9de6ea..6692df830 100644 --- a/Src/Zle/zle_vi.c +++ b/Src/Zle/zle_vi.c @@ -1014,7 +1014,7 @@ int visetbuffer(char **args) { ZLE_INT_T ch; - ZLE_CHAR_T *match = ZWS("_*+"); + const ZLE_CHAR_T *match = ZWS("_*+"); int registermod[] = { MOD_NULL, MOD_PRI, MOD_CLIP }; ZLE_CHAR_T *found; -- 2.43.0 ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 4/6] zle_main: mark statusline as const 2024-01-01 18:10 ` [PATCH 1/6] zle.textobjects: Mark variables as const Jörg Sommer 2024-01-01 18:10 ` [PATCH 2/6] lex: Mark variables with const init " Jörg Sommer 2024-01-01 18:10 ` [PATCH 3/6] zle_vi: " Jörg Sommer @ 2024-01-01 18:10 ` Jörg Sommer 2024-01-01 18:10 ` [PATCH 5/6] module: Mark name argument of some functions const Jörg Sommer 2024-01-01 18:10 ` [PATCH 6/6] zsh: mark hookdef.name as const Jörg Sommer 4 siblings, 0 replies; 10+ messages in thread From: Jörg Sommer @ 2024-01-01 18:10 UTC (permalink / raw) To: zsh-workers; +Cc: Jörg Sommer The statusline points to a const string which content isn't editable. --- Src/Zle/zle_main.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Src/Zle/zle_main.c b/Src/Zle/zle_main.c index 1afb1bf58..de8a05191 100644 --- a/Src/Zle/zle_main.c +++ b/Src/Zle/zle_main.c @@ -154,7 +154,7 @@ mod_export Widget compwidget; /* the status line, a null-terminated metafied string */ /**/ -mod_export char *statusline; +mod_export const char *statusline; /* The current history line and cursor position for the top line * * on the buffer stack. */ -- 2.43.0 ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 5/6] module: Mark name argument of some functions const 2024-01-01 18:10 ` [PATCH 1/6] zle.textobjects: Mark variables as const Jörg Sommer ` (2 preceding siblings ...) 2024-01-01 18:10 ` [PATCH 4/6] zle_main: mark statusline " Jörg Sommer @ 2024-01-01 18:10 ` Jörg Sommer 2024-01-01 18:10 ` [PATCH 6/6] zsh: mark hookdef.name as const Jörg Sommer 4 siblings, 0 replies; 10+ messages in thread From: Jörg Sommer @ 2024-01-01 18:10 UTC (permalink / raw) To: zsh-workers; +Cc: Jörg Sommer --- Src/module.c | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/Src/module.c b/Src/module.c index a6005f30b..b4b5d0a2c 100644 --- a/Src/module.c +++ b/Src/module.c @@ -356,7 +356,7 @@ finish_(UNUSED(Module m)) /**/ void -register_module(char *n, Module_void_func setup, +register_module(const char *n, Module_void_func setup, Module_features_func features, Module_enables_func enables, Module_void_func boot, @@ -846,7 +846,7 @@ Hookdef hooktab; /**/ Hookdef -gethookdef(char *n) +gethookdef(const char *n) { Hookdef p; @@ -974,7 +974,7 @@ deletehookdeffunc(Hookdef h, Hookfn f) /**/ mod_export int -deletehookfunc(char *n, Hookfn f) +deletehookfunc(const char *n, Hookfn f) { Hookdef h = gethookdef(n); @@ -1766,7 +1766,7 @@ dyn_finish_module(Module m) #else static Module_generic_func -module_func(Module m, char *name) +module_func(Module m, const char *name) { #ifdef DYNAMIC_NAME_CLASH_OK return (Module_generic_func) dlsym(m->u.handle, name); @@ -2443,7 +2443,7 @@ bin_zmodload(char *nam, char **args, Options ops, UNUSED(int func)) int ops_au = OPT_ISSET(ops,'a') || OPT_ISSET(ops,'u'); int ret = 1, autoopts; /* options only allowed with -F */ - char *fonly = "lP", *fp; + const char *fonly = "lP", *fp; if (ops_bcpf && !ops_au) { zwarnnam(nam, "-b, -c, -f, and -p must be combined with -a or -u"); @@ -3182,7 +3182,7 @@ bin_zmodload_features(const char *nam, char **args, Options ops) } else if (OPT_ISSET(ops, 'L')) printf("zmodload -F %s ", m->node.nam); for (fp = features, ep = enables; *fp; fp++, ep++) { - char *onoff; + const char *onoff; int term; if (*args) { char **argp; @@ -3452,7 +3452,8 @@ autofeatures(const char *cmdnam, const char *module, char **features, defm = NULL; for (; *features; features++) { - char *fnam, *typnam, *feature; + char *fnam, *feature; + const char *typnam; int add, fchar, flags = defflags; autofeaturefn_t fn; -- 2.43.0 ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 6/6] zsh: mark hookdef.name as const 2024-01-01 18:10 ` [PATCH 1/6] zle.textobjects: Mark variables as const Jörg Sommer ` (3 preceding siblings ...) 2024-01-01 18:10 ` [PATCH 5/6] module: Mark name argument of some functions const Jörg Sommer @ 2024-01-01 18:10 ` Jörg Sommer 4 siblings, 0 replies; 10+ messages in thread From: Jörg Sommer @ 2024-01-01 18:10 UTC (permalink / raw) To: zsh-workers; +Cc: Jörg Sommer At least *zle_main* uses const strings to initialize its structure *zlehooks*. --- Src/zsh.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Src/zsh.h b/Src/zsh.h index a0243e98e..fae62b8d0 100644 --- a/Src/zsh.h +++ b/Src/zsh.h @@ -1578,7 +1578,7 @@ typedef int (*Hookfn) _((Hookdef, void *)); struct hookdef { Hookdef next; - char *name; + const char *name; Hookfn def; int flags; LinkList funcs; -- 2.43.0 ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2024-01-28 0:18 UTC | newest] Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2024-01-01 18:10 Extend usage of const char* Jörg Sommer 2024-01-01 18:10 ` [PATCH 1/6] zle.textobjects: Mark variables as const Jörg Sommer 2024-01-01 18:10 ` [PATCH 2/6] lex: Mark variables with const init " Jörg Sommer 2024-01-26 7:39 ` Oliver Kiddle 2024-01-27 6:41 ` Jörg Sommer 2024-01-28 0:18 ` Oliver Kiddle 2024-01-01 18:10 ` [PATCH 3/6] zle_vi: " Jörg Sommer 2024-01-01 18:10 ` [PATCH 4/6] zle_main: mark statusline " Jörg Sommer 2024-01-01 18:10 ` [PATCH 5/6] module: Mark name argument of some functions const Jörg Sommer 2024-01-01 18:10 ` [PATCH 6/6] zsh: mark hookdef.name as const Jörg Sommer
Code repositories for project(s) associated with this public inbox https://git.vuxu.org/mirror/zsh/ This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).