Re: [Qemu-devel] [PATCH] qtest: ask endianness of the target in qtest_init()

[Peter Maydell]

On 7 October 2016 at 08:31, Greg Kurz <address@hidden> wrote:
> On Fri, 7 Oct 2016 09:10:14 +0200
> Laurent Vivier <address@hidden> wrote:
>
>> On 06/10/2016 22:45, Peter Maydell wrote:
>> > On 6 October 2016 at 11:56, Laurent Vivier <address@hidden> 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