From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751685AbdBUJdw (ORCPT ); Tue, 21 Feb 2017 04:33:52 -0500 Received: from mail-pg0-f66.google.com ([74.125.83.66]:36391 "EHLO mail-pg0-f66.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751624AbdBUJdo (ORCPT ); Tue, 21 Feb 2017 04:33:44 -0500 Date: Tue, 21 Feb 2017 17:33:33 +0800 From: Cheah Kok Cheong To: Ian Abbott Cc: hsweeten@visionengravers.com, gregkh@linuxfoundation.org, devel@driverdev.osuosl.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH] Staging: comedi: drivers: comedi_test: Avoid multiple line dereference Message-ID: <20170221093333.GA2775@linux-Precision-WorkStation-T5500> References: <1487579315-14775-1-git-send-email-thrust73@gmail.com> <20170220160205.GA4017@linux-Precision-WorkStation-T5500> <675e00f6-2e6c-e486-898d-b942c76e26a0@mev.co.uk> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <675e00f6-2e6c-e486-898d-b942c76e26a0@mev.co.uk> User-Agent: Mutt/1.5.24 (2015-08-30) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Mon, Feb 20, 2017 at 05:36:52PM +0000, Ian Abbott wrote: > On 20/02/17 16:02, Cheah Kok Cheong wrote: > >On Mon, Feb 20, 2017 at 10:03:39AM +0000, Ian Abbott wrote: > >>On 20/02/17 08:28, Cheah Kok Cheong wrote: > >>>Fix checkpatch warning "Avoid multiple line dereference" > >>>using a local variable to avoid line wrap. > >>> > >>>Signed-off-by: Cheah Kok Cheong > >>>--- > >>>drivers/staging/comedi/drivers/comedi_test.c | 6 ++---- > >>>1 file changed, 2 insertions(+), 4 deletions(-) > >>> > >>>diff --git a/drivers/staging/comedi/drivers/comedi_test.c b/drivers/staging/comedi/drivers/comedi_test.c > >>>index 2a063f0..fde83e0 100644 > >>>--- a/drivers/staging/comedi/drivers/comedi_test.c > >>>+++ b/drivers/staging/comedi/drivers/comedi_test.c > >>>@@ -480,11 +480,9 @@ static void waveform_ao_timer(unsigned long arg) > >>> /* output the last scan */ > >>> for (i = 0; i < cmd->scan_end_arg; i++) { > >>> unsigned int chan = CR_CHAN(cmd->chanlist[i]); > >>>+ unsigned short d = devpriv->ao_loopbacks[chan]; > >>> > >>>- if (comedi_buf_read_samples(s, > >>>- &devpriv-> > >>>- ao_loopbacks[chan], > >>>- 1) == 0) { > >>>+ if (!comedi_buf_read_samples(s, &d, 1)) { > >>> /* unexpected underrun! (cancelled?) */ > >>> async->events |= COMEDI_CB_OVERFLOW; > >>> goto underrun; > >>> > >> > >>NAK. This leaves devpriv->ao_loopbacks[chan] unchanged. > >> > > > >Thanks for pointing this out. In that case will assigning the variable to > >devpriv->ao_loopbacks[chan] be acceptable? Please review below snippet. > > > >Otherwise I'll just drop the variable and adjust the lines to avoid > >checkpatch warning. > > > >Sorry for the inconvenience caused. > > > >[ Snip ] > > > > /* output the last scan */ > > for (i = 0; i < cmd->scan_end_arg; i++) { > > unsigned int chan = CR_CHAN(cmd->chanlist[i]); > > unsigned short data; > > > > if (!comedi_buf_read_samples(s, &data, 1)) { > > /* unexpected underrun! (cancelled?) */ > > async->events |= COMEDI_CB_OVERFLOW; > > goto underrun; > > } > > > > devpriv->ao_loopbacks[chan] = data; > > } > > /* advance time of last scan */ > > > >[ Snip ] > > It will work, but you could just use a pointer variable set to > &devpriv->ao_loopbacks[chan] and pass that to comedi_buf_read_samples(). > Thanks for the suggestion. I tried below snippet 1 with the shortest pointer name but 80 characters is exceeded. The declaration and initialisation will have to be splitted. Will this be acceptable or am I doing it wrong again? Sorry for the trouble. Snippet 1: [ Snip ] /* output the last scan */ for (i = 0; i < cmd->scan_end_arg; i++) { unsigned int chan = CR_CHAN(cmd->chanlist[i]); unsigned short *p = &devpriv->ao_loopbacks[chan]; if (!comedi_buf_read_samples(s, p, 1)) { /* unexpected underrun! (cancelled?) */ async->events |= COMEDI_CB_OVERFLOW; goto underrun; } } /* advance time of last scan */ [ Snip ] Snippet 2: [ Snip ] /* output the last scan */ for (i = 0; i < cmd->scan_end_arg; i++) { unsigned int chan = CR_CHAN(cmd->chanlist[i]); unsigned short *pd; pd = &devpriv->ao_loopbacks[chan]; if (!comedi_buf_read_samples(s, pd, 1)) { /* unexpected underrun! (cancelled?) */ async->events |= COMEDI_CB_OVERFLOW; goto underrun; } } [ Snip ] Thks. Brgds, CheahKC