Messages in this thread | | | Date | Wed, 16 May 2018 08:26:38 +1000 | From | "Tobin C. Harding" <> | Subject | Re: [PATCH v4 2/3] random: Return nbytes filled from hw RNG |
| |
On Tue, May 15, 2018 at 05:35:46PM -0400, Steven Rostedt wrote: > 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}.
Awesome, cheers.
| |