From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S933031AbdKAJzL (ORCPT ); Wed, 1 Nov 2017 05:55:11 -0400 Received: from usa-sjc-mx-foss1.foss.arm.com ([217.140.101.70]:44850 "EHLO foss.arm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754425AbdKAJzJ (ORCPT ); Wed, 1 Nov 2017 05:55:09 -0400 Subject: Re: [PATCH 02/17] coresight tmc: Hide trace buffer handling for file read To: Julien Thierry , linux-arm-kernel@lists.infradead.org Cc: mathieu.poirier@linaro.org, coresight@lists.linaro.org, linux-kernel@vger.kernel.org, rob.walker@arm.com, mike.leach@linaro.org References: <20171019171553.24056-1-suzuki.poulose@arm.com> <20171019171553.24056-3-suzuki.poulose@arm.com> <190b4a4a-6d54-51e5-52d3-2a313fea714d@arm.com> From: Suzuki K Poulose Message-ID: Date: Wed, 1 Nov 2017 09:55:06 +0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.3.0 MIME-Version: 1.0 In-Reply-To: <190b4a4a-6d54-51e5-52d3-2a313fea714d@arm.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 8bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 20/10/17 13:34, Julien Thierry wrote: > Hi Suzuki, > > On 19/10/17 18:15, Suzuki K Poulose wrote: >> At the moment we adjust the buffer pointers for reading the trace >> data via misc device in the common code for ETF/ETB and ETR. Since >> we are going to change how we manage the buffer for ETR, let us >> move the buffer manipulation to the respective driver files, hiding >> it from the common code. We do so by adding type specific helpers >> for finding the length of data and the pointer to the buffer, >> for a given length at a file position. >> >> Cc: Mathieu Poirier >> Signed-off-by: Suzuki K Poulose >> --- >>   drivers/hwtracing/coresight/coresight-tmc-etf.c | 16 ++++++++++++ >>   drivers/hwtracing/coresight/coresight-tmc-etr.c | 33 ++++++++++++++++++++++++ >>   drivers/hwtracing/coresight/coresight-tmc.c     | 34 ++++++++++++++----------- >>   drivers/hwtracing/coresight/coresight-tmc.h     |  4 +++ >>   4 files changed, 72 insertions(+), 15 deletions(-) >> >> diff --git a/drivers/hwtracing/coresight/coresight-tmc-etf.c b/drivers/hwtracing/coresight/coresight-tmc-etf.c >> index e2513b786242..0b6f1eb746de 100644 >> --- a/drivers/hwtracing/coresight/coresight-tmc-etf.c >> +++ b/drivers/hwtracing/coresight/coresight-tmc-etf.c >> @@ -120,6 +120,22 @@ static void tmc_etf_disable_hw(struct tmc_drvdata *drvdata) >>       CS_LOCK(drvdata->base); >>   } >> +/* >> + * Return the available trace data in the buffer from @pos, with >> + * a maximum limit of @len, updating the @bufpp on where to >> + * find it. >> + */ >> +ssize_t tmc_etb_get_sysfs_trace(struct tmc_drvdata *drvdata, >> +                  loff_t pos, size_t len, char **bufpp) >> +{ >> +    /* Adjust the len to available size @pos */ >> +    if (pos + len > drvdata->len) >> +        len = drvdata->len - pos; >> +    if (len > 0) > > Do we have some guarantee that "pos <= drvdata->len"? because since len is unsigned this check only covers the case were len is 0. > > Maybe it would be better to use a signed variable to store the result of the difference. > >> +        *bufpp = drvdata->buf + pos; >> + * Return the available trace data in the buffer @pos, with a maximum >> + * limit of @len, also updating the @bufpp on where to find it. >> + */ >> +ssize_t tmc_etr_get_sysfs_trace(struct tmc_drvdata *drvdata, >> +                loff_t pos, size_t len, char **bufpp) >> +{ >> +    char *bufp = drvdata->buf + pos; >> +    char *bufend = (char *)(drvdata->vaddr + drvdata->size); >> + >> +    /* Adjust the len to available size @pos */ >> +    if (pos + len > drvdata->len) >> +        len = drvdata->len - pos; >> + >> +    if (len <= 0) >> +        return len; > > Similar issue here. Thanks for spotting. I will fix it. Cheers