[lkml]   [2018]   [Jun]   [12]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
Messages in this thread
SubjectRe: [PATCH] staging: comedi: Improved readability of function comedi_nsamples_left.
You haven't been sending the v2 and v3 patches in the right way.

Take a look through the email archive and see how this is normally done.
Also google it:

On Sun, Jun 10, 2018 at 12:30:01PM +0200, chris wrote:
> Hi Greg,
> I've added changelog text to the patch below. Appreciate your
> feedback!
> Regards,
> Chris Opperman
> >8----------------------------------------------------------------------8<
> Improved the readability of comedi_nsamples_left:
> a) Reduced nesting by using more return calls.

nit: return isn't a function so it can't be called.

> b) Separated variable declarations from code.

It was separate in the original code too.

> c) Using kernel integer types.

The original types were fine/better.

The rules are that we don't like uint32_t and the reason for not liking
it is that we looked through the kernel code and standardized on what
was most common...

Generally u32 and friends mean that the hardware or network protocol
specifies the number of bits. That's especially true for s32. If you
see code using s32 when it's not tied to a hardware spec then that's a
sign that the code is written by an insane person and it's a tricky
situation to deal with.

Some other reasons to use u32 is because it's shorter to type than
unsigned int and because of line lengths... I've sometimes done that.
But it's perhaps lazy.

> Signed-off-by: Chris Opperman <>
> ---
> drivers/staging/comedi/drivers.c | 29 ++++++++++++++---------------
> 1 file changed, 14 insertions(+), 15 deletions(-)
> diff --git a/drivers/staging/comedi/drivers.c b/drivers/staging/comedi/drivers.c
> index 9d73347..3207ae2 100644
> --- a/drivers/staging/comedi/drivers.c
> +++ b/drivers/staging/comedi/drivers.c
> @@ -468,26 +468,25 @@ EXPORT_SYMBOL_GPL(comedi_nscans_left);
> * Returns the number of samples remaining to complete the command, or the
> * specified expected number of samples (@nsamples), whichever is fewer.
> */
> -unsigned int comedi_nsamples_left(struct comedi_subdevice *s,
> - unsigned int nsamples)
> +u32 comedi_nsamples_left(struct comedi_subdevice *s, u32 nsamples)
> {
> struct comedi_async *async = s->async;
> struct comedi_cmd *cmd = &async->cmd;
> + u32 scans_left;

I would make scans_left an unsigned long long so that you can remove
the cast when you do the multiply.

> + u64 samples_left;

Nothing wrong with unsigned long long.

dan carpenter

 \ /
  Last update: 2018-06-12 10:57    [W:0.047 / U:1.328 seconds]
©2003-2018 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site