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=-4.1 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,MAILING_LIST_MULTI,SPF_HELO_NONE, SPF_PASS 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 45B1EC388F9 for ; Wed, 11 Nov 2020 18:01:42 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id CAE792072C for ; Wed, 11 Nov 2020 18:01:38 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (1024-bit key) header.d=kernel.org header.i=@kernel.org header.b="rhB4e5ze" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726525AbgKKSBg (ORCPT ); Wed, 11 Nov 2020 13:01:36 -0500 Received: from mail.kernel.org ([198.145.29.99]:41596 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1725966AbgKKSBa (ORCPT ); Wed, 11 Nov 2020 13:01:30 -0500 Received: from quaco.ghostprotocols.net (unknown [179.97.37.151]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPSA id 052A5206B6; Wed, 11 Nov 2020 18:01:30 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1605117690; bh=rWIlCSVLkxy6U/Oi8BLJipuAg6hfbgdqGmwN3XRb5NQ=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=rhB4e5zezCUflGLHAy5npSm00xFopJIbgN4b5ju8T3p34nghdDlSzKKE85MLRAAns hb6VapXatmNnYp6avXw33gpt2kbM9b1++XJOUHby6vVXMfSBDDSgMv1RtMqEMNH+AM 995Y4fU95RHGjpV3xK1/f0cEw0fiX1B90AMIzehc= Received: by quaco.ghostprotocols.net (Postfix, from userid 1000) id 29330411D1; Wed, 11 Nov 2020 15:01:27 -0300 (-03) Date: Wed, 11 Nov 2020 15:01:27 -0300 From: Arnaldo Carvalho de Melo To: Dave Martin Cc: Andre Przywara , "leo.yan@linaro.org" , James Clark , Peter Zijlstra , Ingo Molnar , Mark Rutland , Alexander Shishkin , Jiri Olsa , Namhyung Kim , Al Grant , Wei Li , John Garry , Will Deacon , Mathieu Poirier , "linux-kernel@vger.kernel.org" Subject: Re: [PATCH v8 06/22] perf arm-spe: Refactor printing string to buffer Message-ID: <20201111180127.GD380127@kernel.org> References: <20201111071149.815-1-leo.yan@linaro.org> <20201111071149.815-7-leo.yan@linaro.org> <20201111153555.GG355344@kernel.org> <20201111173922.GA380127@kernel.org> <20201111175827.GR6882@arm.com> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <20201111175827.GR6882@arm.com> X-Url: http://acmel.wordpress.com Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Em Wed, Nov 11, 2020 at 05:58:27PM +0000, Dave Martin escreveu: > > On Wed, Nov 11, 2020 at 05:39:22PM +0000, Arnaldo Carvalho de Melo wrote: > > Em Wed, Nov 11, 2020 at 03:45:23PM +0000, Andr� Przywara escreveu: > > > On 11/11/2020 15:35, Arnaldo Carvalho de Melo wrote: > > > > Isn't this 'ret +=' ? Otherwise if any of these arm_spe_pkt_snprintf() > > > > calls are made the previous 'ret' value is simply discarded. Can you > > > > clarify this? > > > ret is the same as err. If err is negative (from previous calls), we > > > return that straight away, so it does nothing but propagating the error. > > Usually the return of a snprintf is used to account for buffer space, ok > > I'll have to read it, which I shouldn't as snprintf has a well defined > > meaning... > > Ok, now that I look at it, I realize it is not a snprintf() routine, but > > something with different semantics, that will look at a pointer to an > > integer and then do nothing if it comes with some error, etc, confusing > > :-/ > Would you be happier if the function were renamed? > Originally we were aiming for snprintf() semantics, but this still > spawns a lot of boilerplate code and encourages mistakes in the local > caller here -- hence the current sticky error approach. > So maybe the name should now be less "snprintf"-like. Please, its important to stick to semantics for such well known type of routines, helps reviewing, etc. I'll keep the series up to that point and will run my build tests, then push it publicly to acme/perf/core and you can go from there, ok? I've changed the BIT() to BIT_ULL() as Andre suggested and I'm testing it again. - Arnaldo