On 08/18/2017 04:46 PM, Philippe Mathieu-Daudé wrote: > On 08/18/2017 06:15 PM, Eric Blake wrote: >> There was only one caller; it's easier to inline things. > > What's the benefit of this change? Having a separate function ease code > reading. You might add the 'inline' qualifier but the compiler is smart > enough to inline it. > > Instead of this change I'd inline it and remove the return value, having > a void function initializing s->big_endian. Matter of taste I think :) > >> >> Signed-off-by: Eric Blake <ebl...@redhat.com> >> --- >> tests/libqtest.c | 22 ++++++---------------- >> 1 file changed, 6 insertions(+), 16 deletions(-)
I think the diffstat shows that inlining is a win. My other reason that it is a win: >> @@ -351,8 +338,11 @@ QTestState >> *qtest_init_without_qmp_handshake(const char *extra_args) >> } >> >> /* ask endianness of the target */ >> - >> - s->big_endian = qtest_query_target_endianness(s); >> + qtest_sendf(s, "endianness\n"); >> + args = qtest_rsp(s, 1); The old code has to pass an arbitrary QTestState *s down to a helper function. But I'm trying to get rid of as many functions as possible that depend on arbitrary QTestState *s, particularly when callers end up passing 'global_qtest' for 's'. Inlining it makes it more obvious that the changes in 10/13 are safely operating on global_qtest, which in turn makes it possible for 12/13 to drop QTestState *s parameters from qtest_sendf() and qtest_rsp(). If the function were not inlined, those later cleanups would not be as trivial. (Or put another way - I rebased this patch to be earlier in the series precisely because of what I ran into while writing those patches) -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org
signature.asc
Description: OpenPGP digital signature