[PATCH v4] staging: comedi: Improved readability of function comedi_nsamples_left.

[PATCH v4] staging: comedi: Improved readability of function comedi_nsamples_left.

[Chris Opperman]

Changes since v3:
a) Reverted u64 to unsigned long long and u32 to unsigned int.
b) Added patch versioning.
c) Changed type of scans_left to unsigned long long to avoid cast.
d) Clarified and updated changelog.

>8-----------------------------------------------------------------------8<

Improve readability of comedi_nsamples_left:
a) Reduce nesting by using more return statements.
b) Declare variables scans_left and samples_left at start of function.
c) Change type of scans_Left to unsigned long long to avoid cast.

Signed-off-by: Chris Opperman <eklikeroomys@gmail.com>
---
 drivers/staging/comedi/drivers.c | 26 +++++++++++++-------------
 1 file changed, 13 insertions(+), 13 deletions(-)

diff --git a/drivers/staging/comedi/drivers.c b/drivers/staging/comedi/drivers.c
index 9d73347..57dd63d 100644
--- a/drivers/staging/comedi/drivers.c
+++ b/drivers/staging/comedi/drivers.c
@@ -473,21 +473,21 @@ unsigned int comedi_nsamples_left(struct comedi_subdevice *s,
 {
 	struct comedi_async *async = s->async;
 	struct comedi_cmd *cmd = &async->cmd;
+	unsigned long long scans_left;
+	unsigned long long samples_left;

-	if (cmd->stop_src == TRIG_COUNT) {
-		unsigned int scans_left = __comedi_nscans_left(s, cmd->stop_arg);
-		unsigned int scan_pos =
-		    comedi_bytes_to_samples(s, async->scan_progress);
-		unsigned long long samples_left = 0;
-
-		if (scans_left) {
-			samples_left = ((unsigned long long)scans_left *
-					cmd->scan_end_arg) - scan_pos;
-		}
+	if (cmd->stop_src != TRIG_COUNT)
+		return nsamples;

-		if (samples_left < nsamples)
-			nsamples = samples_left;
-	}
+	scans_left = __comedi_nscans_left(s, cmd->stop_arg);
+	if (!scans_left)
+		return 0;
+
+	samples_left = scans_left * cmd->scan_end_arg -
+		comedi_bytes_to_samples(s, async->scan_progress);
+
+	if (samples_left < nsamples)
+		return samples_left;
 	return nsamples;
 }
 EXPORT_SYMBOL_GPL(comedi_nsamples_left);
--
2.1.4

Re: [PATCH v4] staging: comedi: Improved readability of function comedi_nsamples_left.

[Dan Carpenter]

Sooooooooo close...

On Tue, Jun 12, 2018 at 11:09:44PM +0200, Chris Opperman wrote:
> Changes since v3:
> a) Reverted u64 to unsigned long long and u32 to unsigned int.
> b) Added patch versioning.
> c) Changed type of scans_left to unsigned long long to avoid cast.
> d) Clarified and updated changelog.
> 
> >8-----------------------------------------------------------------------8<

This part here needs to go ...

> 
> Improve readability of comedi_nsamples_left:
> a) Reduce nesting by using more return statements.
> b) Declare variables scans_left and samples_left at start of function.
> c) Change type of scans_Left to unsigned long long to avoid cast.
                          ^
> 
> Signed-off-by: Chris Opperman <eklikeroomys@gmail.com>
> ---

... down here, under the --- cut off line.

>  drivers/staging/comedi/drivers.c | 26 +++++++++++++-------------
>  1 file changed, 13 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/staging/comedi/drivers.c b/drivers/staging/comedi/drivers.c

regards,
dan carpenter

Re: [PATCH v4] staging: comedi: Improved readability of function comedi_nsamples_left.

[Chris Opperman]

Hi Dan/Ian,

Noted your comments regarding additional text, thanks! Just curious whether 
the "scissors" format given at the link below is valid? 

https://kernelnewbies.org/PatchTipsAndTricks

It is given as an alternative to placing additional text below the
cut-off line.

Kind Regards,
Chris Opperman

Re: [PATCH v4] staging: comedi: Improved readability of function comedi_nsamples_left.

[Ian Abbott]

On 13/06/18 19:26, Chris Opperman wrote:
> Hi Dan/Ian,
> 
> Noted your comments regarding additional text, thanks! Just curious whether
> the "scissors" format given at the link below is valid?
> 
> https://kernelnewbies.org/PatchTipsAndTricks
> 
> It is given as an alternative to placing additional text below the
> cut-off line.

I don't know.  Maybe?  I haven't seen it used before.

-- 
-=( Ian Abbott <abbotti@mev.co.uk> || Web: www.mev.co.uk )=-
-=( MEV Ltd. is a company registered in England & Wales. )=-
-=( Registered number: 02862268.  Registered address:    )=-
-=( 15 West Park Road, Bramhall, STOCKPORT, SK7 3JZ, UK. )=-

Re: [PATCH v4] staging: comedi: Improved readability of function comedi_nsamples_left.

[Dan Carpenter]

On Wed, Jun 13, 2018 at 08:26:43PM +0200, Chris Opperman wrote:
> Hi Dan/Ian,
> 
> Noted your comments regarding additional text, thanks! Just curious whether 
> the "scissors" format given at the link below is valid? 
> 
> https://kernelnewbies.org/PatchTipsAndTricks
> 
> It is given as an alternative to placing additional text below the
> cut-off line.
> 

Try it yourself.  Save your email as email.txt and then
`cat email.txt | git am` and then review the patch with git log -p.

I've seen people do the scissors thing, but I assume the maintainer has
to hand edit the log which we refuse to do.

regards,
dan carpenter

Re: [PATCH v4] staging: comedi: Improved readability of function comedi_nsamples_left.

[Ian Abbott]

On 13/06/18 20:05, Dan Carpenter wrote:
> On Wed, Jun 13, 2018 at 08:26:43PM +0200, Chris Opperman wrote:
>> Hi Dan/Ian,
>>
>> Noted your comments regarding additional text, thanks! Just curious whether
>> the "scissors" format given at the link below is valid?
>>
>> https://kernelnewbies.org/PatchTipsAndTricks
>>
>> It is given as an alternative to placing additional text below the
>> cut-off line.
>>
> 
> Try it yourself.  Save your email as email.txt and then
> `cat email.txt | git am` and then review the patch with git log -p.
> 
> I've seen people do the scissors thing, but I assume the maintainer has
> to hand edit the log which we refuse to do.

It can be done automatically with the git am -c or --scissors option, or 
by setting mailinfo.scissors to true in the git config.  But since the 
use of these special scissors lines is not documented in 
Documentation/process/submitting-patches.rst, I don't think it is safe 
to assume that no manual intervention would be required by the 
maintainer to deal with it.

I wonder where the "scissors" advice on the kernelnewbies page came from?

-- 
-=( Ian Abbott <abbotti@mev.co.uk> || Web: www.mev.co.uk )=-
-=( MEV Ltd. is a company registered in England & Wales. )=-
-=( Registered number: 02862268.  Registered address:    )=-
-=( 15 West Park Road, Bramhall, STOCKPORT, SK7 3JZ, UK. )=-

Re: [PATCH v4] staging: comedi: Improved readability of function comedi_nsamples_left.

[Chris Opperman]

Hi Ian/Dan,

In that case I'll stick to the cut-off line format in future. Thanks!

Kind Regards,
Chris Opperman

Re: [PATCH v4] staging: comedi: Improved readability of function comedi_nsamples_left.

[Ian Abbott]

On 12/06/18 22:09, Chris Opperman wrote:
> Changes since v3:
> a) Reverted u64 to unsigned long long and u32 to unsigned int.
> b) Added patch versioning.
> c) Changed type of scans_left to unsigned long long to avoid cast.
> d) Clarified and updated changelog.
> 
>> 8-----------------------------------------------------------------------8<
> 
> Improve readability of comedi_nsamples_left:
> a) Reduce nesting by using more return statements.
> b) Declare variables scans_left and samples_left at start of function.
> c) Change type of scans_Left to unsigned long long to avoid cast.
> 
> Signed-off-by: Chris Opperman <eklikeroomys@gmail.com>
> ---
>   drivers/staging/comedi/drivers.c | 26 +++++++++++++-------------
>   1 file changed, 13 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/staging/comedi/drivers.c b/drivers/staging/comedi/drivers.c
> index 9d73347..57dd63d 100644
> --- a/drivers/staging/comedi/drivers.c
> +++ b/drivers/staging/comedi/drivers.c
> @@ -473,21 +473,21 @@ unsigned int comedi_nsamples_left(struct comedi_subdevice *s,
>   {
>   	struct comedi_async *async = s->async;
>   	struct comedi_cmd *cmd = &async->cmd;
> +	unsigned long long scans_left;
> +	unsigned long long samples_left;
> 
> -	if (cmd->stop_src == TRIG_COUNT) {
> -		unsigned int scans_left = __comedi_nscans_left(s, cmd->stop_arg);
> -		unsigned int scan_pos =
> -		    comedi_bytes_to_samples(s, async->scan_progress);
> -		unsigned long long samples_left = 0;
> -
> -		if (scans_left) {
> -			samples_left = ((unsigned long long)scans_left *
> -					cmd->scan_end_arg) - scan_pos;
> -		}
> +	if (cmd->stop_src != TRIG_COUNT)
> +		return nsamples;
> 
> -		if (samples_left < nsamples)
> -			nsamples = samples_left;
> -	}
> +	scans_left = __comedi_nscans_left(s, cmd->stop_arg);
> +	if (!scans_left)
> +		return 0;
> +
> +	samples_left = scans_left * cmd->scan_end_arg -
> +		comedi_bytes_to_samples(s, async->scan_progress);
> +
> +	if (samples_left < nsamples)
> +		return samples_left;
>   	return nsamples;
>   }
>   EXPORT_SYMBOL_GPL(comedi_nsamples_left);
> --
> 2.1.4
> 

The code changes look fine.  You just need to correct the commit 
message, as pointed out by Dan Carpenter.

For reference, everything below the "---" cut-off line gets stripped out 
of the commit message in the git repository, so that is a good place to 
add comments about the patch itself (such as change logs) that do not 
belong in the commit message.

-- 
-=( Ian Abbott <abbotti@mev.co.uk> || Web: www.mev.co.uk )=-
-=( MEV Ltd. is a company registered in England & Wales. )=-
-=( Registered number: 02862268.  Registered address:    )=-
-=( 15 West Park Road, Bramhall, STOCKPORT, SK7 3JZ, UK. )=-