From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-7.2 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,NICE_REPLY_A,SPF_HELO_NONE, SPF_PASS,USER_AGENT_SANE_1 autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 69358C433EF for ; Mon, 13 Sep 2021 10:23:52 +0000 (UTC) Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 0DD4660FA0 for ; Mon, 13 Sep 2021 10:23:52 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org 0DD4660FA0 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=csgraf.de Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=nongnu.org Received: from localhost ([::1]:47728 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1mPj7v-0003qb-9U for qemu-devel@archiver.kernel.org; Mon, 13 Sep 2021 06:23:51 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:47832) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1mPir5-0007mi-PP; Mon, 13 Sep 2021 06:06:32 -0400 Received: from mail.csgraf.de ([85.25.223.15]:43850 helo=zulu616.server4you.de) by eggs.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1mPir3-0003EU-Ll; Mon, 13 Sep 2021 06:06:27 -0400 Received: from MacBook-Air.alex.local (dynamic-095-118-088-150.95.118.pool.telefonica.de [95.118.88.150]) by csgraf.de (Postfix) with ESMTPSA id E2807608037D; Mon, 13 Sep 2021 12:06:22 +0200 (CEST) Subject: Re: [PATCH v8 16/19] hvf: arm: Implement PSCI handling To: Richard Henderson , Peter Maydell References: <20210519202253.76782-1-agraf@csgraf.de> <20210519202253.76782-17-agraf@csgraf.de> <11281306-a11a-5b8c-6b2b-202be8e2655a@linaro.org> <8e0879b6-23c0-8f14-fd96-4d72f1d640c1@linaro.org> From: Alexander Graf Message-ID: <6fe284da-6989-f837-adda-1aa722f0d345@csgraf.de> Date: Mon, 13 Sep 2021 12:06:20 +0200 User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:78.0) Gecko/20100101 Thunderbird/78.14.0 MIME-Version: 1.0 In-Reply-To: <8e0879b6-23c0-8f14-fd96-4d72f1d640c1@linaro.org> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit Content-Language: en-US Received-SPF: pass client-ip=85.25.223.15; envelope-from=agraf@csgraf.de; helo=zulu616.server4you.de X-Spam_score_int: -54 X-Spam_score: -5.5 X-Spam_bar: ----- X-Spam_report: (-5.5 / 5.0 requ) BAYES_00=-1.9, NICE_REPLY_A=-3.584, SPF_HELO_NONE=0.001, SPF_PASS=-0.001 autolearn=ham autolearn_force=no X-Spam_action: no action X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Eduardo Habkost , =?UTF-8?Q?Philippe_Mathieu-Daud=c3=a9?= , QEMU Developers , Cameron Esfahani , Roman Bolshakov , qemu-arm , Frank Yang , Paolo Bonzini , Peter Collingbourne Errors-To: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Sender: "Qemu-devel" On 12.09.21 23:40, Richard Henderson wrote: > On 9/12/21 2:37 PM, Alexander Graf wrote: >> >> On 12.09.21 23:20, Richard Henderson wrote: >>> On 9/12/21 1:36 PM, Alexander Graf wrote: >>>>> I think the callsites would be clearer if you made the function >>>>> return true for "PSCI call handled", false for "not recognised, >>>>> give the guest an UNDEF". Code like >>>>>            if (hvf_handle_psci_call(cpu)) { >>>>>                stuff; >>>>>            } >>>>> >>>>> looks like the 'stuff' is for the "psci call handled" case, >>>>> which at the moment it isn't. >>>> >>>> >>>> This function merely follows standard C semantics along the lines >>>> of "0 >>>> means success, !0 is error". Isn't that what you would usually expect? >>> >>> No, not really.  I expect stuff that returns error codes to return >>> negative integers on failure.  I expect stuff that returns a boolean >>> success/failure to return true on success. >> >> >> Fair, I'll change it to return -1 then. Thanks! > > Not quite the point I was making.  If the only two return values are > -1/0, then bool false/true is in fact more appropriate. If the whole code base adheres to it, maybe. The big problem with using true/false as return values for functions that don't make it very explicit (have an is in their name or use gerund for example). QEMU uses a lot of 0 == success internally. If you now add bools to the a code base that already uses int returns a lot, you always need to look up what a function actually returns if the caller treats it as bool (if, ?, assert, etc), making code review and reasoning about code flows extremely hard. I've had one too many occasions where I called an innocently looking API and put the assert() the wrong way around for example. I don't think we can solve this problem here, but IMHO the only sensible way to get to good readability would be to have functions that return success/failure return a libc defined struct that indicates the error. Then check for success explicitly on every caller site. Overloading bool or int for success/failure return is just bad :). Alex