Thread (2 messages) 2 messages, 2 authors, 2024-05-30

Re: [PATCH 17/19] builtin/rebase: adapt code to not assign string constants to non-const

From: Patrick Steinhardt <hidden>
Date: 2024-05-30 11:31:14

On Wed, May 29, 2024 at 02:01:08PM -0700, Junio C Hamano wrote:
Patrick Steinhardt [off-list ref] writes:
quoted
When computing the rebase strategy we temporarily assign a string
constant to `options.strategy` before we call `xstrdup()` on it.
Furthermore, the default backend is being assigned a string constant via
`REBASE_OPTIONS_INIT`. Both of these will cause warnings once we enable
`-Wwrite-strings`.

Adapt the code such that we only store allocated strings in those
variables.

Signed-off-by: Patrick Steinhardt <redacted>
---
 builtin/rebase.c | 16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)
One gripe I have in this change is that it used to be crystal clear
what the hardcoded default was (i.e. written in the initialization
data), but now you have to follow the program flow to see what the
hardcoded default is.
True. We could adapt the macro to instead `xstrdup()` the default
backend. But that feels a bit hidden, so I don't think this is a
partiuclarly great option.

An alternative would be to wrap initialization into a function
`rebase_options_init()` that instead allocates the default backend
string. We can then also create a `rebase_options_release()` counter
part that releases its resources such that this whole thing becomes a
bit more self contained.

Patrick

Attachments

Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help