On 7 October 2016 at 08:31, Greg Kurz <gr...@kaod.org> wrote: > On Fri, 7 Oct 2016 09:10:14 +0200 > Laurent Vivier <lviv...@redhat.com> wrote: > >> On 06/10/2016 22:45, Peter Maydell wrote: >> > On 6 October 2016 at 11:56, Laurent Vivier <lviv...@redhat.com> wrote: >> >> + /* ask endianness of the target */ >> >> + >> >> + qtest_sendf(s, "endianness\n"); >> >> + args = qtest_rsp(s, 1); >> >> + g_assert(strcmp(args[1], "big") == 0 || strcmp(args[1], "little") == >> >> 0); >> >> + s->big_endian = strcmp(args[1], "big") == 0; >> >> + g_strfreev(args); >> > >> > This would be better in its own utility function, I think. >> >> Yes, I agree, but my wondering was how to name it :P , >> qtest_big_endian() and target_big_endian() are already in use, and as it >> is a 6 lines function, used once, I guessed we could inline it here. >> > > This is TARGET_WORDS_BIGENDIAN which is constant for a single QEMU > run... why moving it to a function ? Unless there are plans to > have dynamic target endianness in QEMU, I guess it makes more > sense to open code as you did.
I thought it would be better in its own function simply because "query the QEMU process for the value of TARGET_WORDS_BIGENDIAN" is a simple well defined and self contained operation, which isn't very tightly coupled to the init-the-connection stuff that qtest_init() does. qtest_init() is already a fairly long function and so I think it makes for more maintainable code to have a (static, local) qtest_query_target_endianness() function rather than inlining it. thanks -- PMM
