lkml.org 
[lkml]   [2018]   [May]   [15]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH v4 2/3] random: Return nbytes filled from hw RNG
On Wed, 16 May 2018 07:17:06 +1000
"Tobin C. Harding" <me@tobin.cc> wrote:

> > > -void get_random_bytes_arch(void *buf, int nbytes)
> > > +int __must_check get_random_bytes_arch(void *buf, int nbytes)
> > > {
> > > char *p = buf;
> > > + int left = nbytes;
> >
> > Just a nit, but I know some kernel devs prefer "upside-down-xmas-tree"
> > style of declarations. Which would make the above:
> >
> > int left = nbytes;
> > char *p = buf;
>
> Super specific coding style and rigorous code cleanliness is a big part
> of why I love kernel dev. Thanks for pointing this one out.

It's a relatively new form, but I like it. It makes the code look "less
messy" ;-) Some devs don't care, others do. This file already breaks
it, so it really is up to you. Like I said, it's "just a nit", not
really important.

>
> While we are on these code lines, whats the typical kernel variable name
> for a loop counter that is going to be counted down? 'left',
> 'remaining', 'to_go', 'still'???

"left" looks good to me.

>
> > >
> > > - trace_get_random_bytes_arch(nbytes, _RET_IP_);
> > > - while (nbytes) {
> > > + trace_get_random_bytes_arch(left, _RET_IP_);
> >
> > Nothing to do with this patch series, but I wonder if we should move
> > the trace event below, and record how much was done.
>
> I don't fully understand trace events, I just left this line in tact
> and hoped for the best :(

Your patch is fine. This could be something to add after your series.

>
> /me adds 'trace events' to list of things to learn more about

Just look at /sys/kernel/debug/tracing/events

Or read Documentation/trace/ftrace.{rst,txt}.


-- Steve

\
 
 \ /
  Last update: 2018-05-15 23:36    [W:0.058 / U:0.964 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site