linux-kernel - Re: [PATCH] staging: comedi: shortened a long line

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Sat, 09 Jun 2018 02:36:31 -0700
From:   Joe Perches <joe@...ches.com>
To:     Chris Opperman <eklikeroomys@...il.com>
Cc:     Ian Abbott <abbotti@....co.uk>,
        H Hartley Sweeten <hsweeten@...ionengravers.com>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Frank Mori Hess <fmh6jj@...il.com>,
        Simo Koskinen <koskisoft@...il.com>,
        devel@...verdev.osuosl.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] staging: comedi: shortened a long line

On Sat, 2018-06-09 at 12:54 +0200, Chris Opperman wrote:
> Shortened a long line to improve readability in 
> drivers/staging/comedi/drivers.c

Hi Chris.

Look at the whole function and see if you can
find a better way to write it instead of merely
doing what a brainless tool like checkpatch asks.

> diff --git a/drivers/staging/comedi/drivers.c b/drivers/staging/comedi/drivers.c
[]
> @@ -475,7 +475,8 @@ unsigned int comedi_nsamples_left(struct comedi_subdevice *s,
>  	struct comedi_cmd *cmd = &async->cmd;
>  
>  	if (cmd->stop_src == TRIG_COUNT) {
> -		unsigned int scans_left = __comedi_nscans_left(s, cmd->stop_arg);
> +		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;

For instance, this is the existing function:

unsigned int comedi_nsamples_left(struct comedi_subdevice *s,
				  unsigned int nsamples)
{
	struct comedi_async *async = s->async;
	struct comedi_cmd *cmd = &async->cmd;

	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 (samples_left < nsamples)
			nsamples = samples_left;
	}
	return nsamples;
}
EXPORT_SYMBOL_GPL(comedi_nsamples_left);

By using multiple returns and removing indentation,
this could become something that doesn't fits
neatly on 80 columns.
It's the same number of vertical lines too.

unsigned int comedi_nsamples_left(struct comedi_subdevice *s,
				  unsigned int nsamples)
{
	struct comedi_async *async = s->async;
	struct comedi_cmd *cmd = &async->cmd;
	unsigned int scans_left;
	u64 samples_left;

	if (cmd->stop_src != TRIG_COUNT)
		return nsamples;

	scans_left = __comedi_nscans_left(s, cmd->stop_arg);
	if (scans_left == 0)
		return 0;

	samples_left = (u64)scans_left * cmd->scan_end_arg -
			comedi_bytes_to_samples(s, s->async->scan_progress);
	if (samples_left < nsamples)
		return samples_left;

	return nsamples;
}
EXPORT_SYMBOL_GPL(comedi_nsamples_left);