On Thu 21 Nov 2019 07:34:45 PM CET, Lukas Straub wrote: >> > diff --git a/block/quorum.c b/block/quorum.c >> > index df68adcfaa..6100d4108a 100644 >> > --- a/block/quorum.c >> > +++ b/block/quorum.c >> > @@ -1054,6 +1054,12 @@ static void quorum_del_child(BlockDriverState *bs, >> > BdrvChild *child, >> > /* We know now that num_children > threshold, so blkverify must be >> > false */ >> > assert(!s->is_blkverify); >> > >> > + unsigned child_id; >> > + sscanf(child->name, "children.%u", &child_id); >> >> sscanf() cannot detect overflow. Do we trust our input enough to >> ignore this shortfall in the interface, or should we be using saner >> interfaces like qemu_strtoul()? For that matter, why do we have to >> reparse something; is it not already available somewhere in numerical >> form? > > Yes, I wondered about that too, but found no other way. But the input > is trusted, AFAIK the only way to add child nodes is trough > quorum_add_child above and quorum_open and there already are adequate > checks there.
I also don't see any other way to get that value, unless we change BDRVQuorumState to store that information (e.g. instead of children being a list of pointers BdrvChild ** it could be a list of {pointer, index}, or something like that). There's another (more convoluted) alternative if we don't want to parse child->name. Since we only want to know if the child number equals s->next_child_index - 1, we can do it the other way around: snprintf(str, 32, "children.%u", s->next_child_index - 1); and then compare str and child->name. Berto