From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:37881) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bsRuZ-0008Uv-Br for qemu-devel@nongnu.org; Fri, 07 Oct 2016 05:57:52 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bsRuT-00033y-B7 for qemu-devel@nongnu.org; Fri, 07 Oct 2016 05:57:50 -0400 Received: from 18.mo6.mail-out.ovh.net ([46.105.73.110]:43655) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bsRuT-00033H-4W for qemu-devel@nongnu.org; Fri, 07 Oct 2016 05:57:45 -0400 Received: from player738.ha.ovh.net (b7.ovh.net [213.186.33.57]) by mo6.mail-out.ovh.net (Postfix) with ESMTP id E7A3135D84 for ; Fri, 7 Oct 2016 11:57:43 +0200 (CEST) Date: Fri, 7 Oct 2016 11:57:38 +0200 From: Greg Kurz Message-ID: <20161007115738.6f5ddbad@bahia> In-Reply-To: References: <1475780218-26393-1-git-send-email-lvivier@redhat.com> <568268e9-678e-3301-d66e-001d5d1a3d4a@redhat.com> <20161007093128.674b861d@bahia> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH] qtest: ask endianness of the target in qtest_init() List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Peter Maydell Cc: Laurent Vivier , QEMU Developers , David Gibson On Fri, 7 Oct 2016 10:36:58 +0100 Peter Maydell wrote: > On 7 October 2016 at 08:31, Greg Kurz wrote: > > On Fri, 7 Oct 2016 09:10:14 +0200 > > Laurent Vivier wrote: > > > >> On 06/10/2016 22:45, Peter Maydell wrote: > >> > On 6 October 2016 at 11:56, Laurent Vivier 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 It's a matter of taste, but your point makes sense. I'd say let the maintainer decide, but... $ ./scripts/get_maintainer.pl -f tests/libqtest.c get_maintainer.pl: No maintainers found, printing recent contributors. get_maintainer.pl: Do not blindly cc: them on patches! Use common sense. Cheers. -- Greg