linux-kernel - Re: [PATCH v4 2/3] random: Return nbytes filled from hw RNG

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 for Android: free password hash cracker in your pocket

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

Message-ID: <20180515173546.6d80b119@gandalf.local.home>
Date:   Tue, 15 May 2018 17:35:46 -0400
From:   Steven Rostedt <rostedt@...dmis.org>
To:     "Tobin C. Harding" <me@...in.cc>
Cc:     Theodore Ts'o <tytso@....edu>,
        Linus Torvalds <torvalds@...ux-foundation.org>,
        Randy Dunlap <rdunlap@...radead.org>,
        Kees Cook <keescook@...omium.org>,
        Anna-Maria Gleixner <anna-maria@...utronix.de>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Arnd Bergmann <arnd@...db.de>, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v4 2/3] random: Return nbytes filled from hw RNG

On Wed, 16 May 2018 07:17:06 +1000
"Tobin C. Harding" <me@...in.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