Message ID | 1576074210-52834-3-git-send-email-pbonzini@redhat.com |
---|---|
State | New |
Headers | show |
Series | migration: fix stringop-truncation warning | expand |
On 12/11/19 6:23 AM, Paolo Bonzini wrote: > @@ -44,8 +44,7 @@ void global_state_store_running(void) > { > const char *state = RunState_str(RUN_STATE_RUNNING); > assert(strlen(state) < sizeof(global_state.runstate)); > - strncpy((char *)global_state.runstate, > - state, sizeof(global_state.runstate)); > + memcpy(global_state.runstate, state, strlen(state) + 1); We should assign the strlen result to a local variable rather than compute it twice. r~
On 12/12/19 01:55, Richard Henderson wrote: > On 12/11/19 6:23 AM, Paolo Bonzini wrote: >> @@ -44,8 +44,7 @@ void global_state_store_running(void) >> { >> const char *state = RunState_str(RUN_STATE_RUNNING); >> assert(strlen(state) < sizeof(global_state.runstate)); >> - strncpy((char *)global_state.runstate, >> - state, sizeof(global_state.runstate)); >> + memcpy(global_state.runstate, state, strlen(state) + 1); > > We should assign the strlen result to a local variable rather than compute it > twice. We could even strcpy since the assertion ensures it's valid, but perhaps it's better to do nothing, since the best alternative would be memset+strcpy, i.e. back to strncpy. We generally are quite mindful about our uses of strncpy, but maybe we could fold the assertion and strncpy into a qemu_strncpy function, and/or qemu_strncpy_nonul for when you're copying into an array that does _not_ need to be nul-terminated (so the assertion can become <= rather than <). I'll add it to BiteSizedTasks. Paolo
diff --git a/migration/global_state.c b/migration/global_state.c index 2531147..a39a876 100644 --- a/migration/global_state.c +++ b/migration/global_state.c @@ -44,8 +44,7 @@ void global_state_store_running(void) { const char *state = RunState_str(RUN_STATE_RUNNING); assert(strlen(state) < sizeof(global_state.runstate)); - strncpy((char *)global_state.runstate, - state, sizeof(global_state.runstate)); + memcpy(global_state.runstate, state, strlen(state) + 1); } bool global_state_received(void)