Re: [PATCH v4 10/12] Add a base implementation of SHA-256 support
"brian m. carlson" writes: > I need to do a reroll to address some other issues people brought up, so > I can remove this line. OK. I actually do not feel too strongly about this one, by the way.
Re: [PATCH v4 10/12] Add a base implementation of SHA-256 support
On Mon, Oct 29, 2018 at 09:39:33AM +0900, Junio C Hamano wrote: > "brian m. carlson" writes: > > This may not strictly be needed, but removing it makes the header no > > longer self-contained, which blows up my (and others') in-editor > > linting. > > That sounds like bending backwards to please tools, though. Can't > these in-editor linting learn the local rules of the codebase they > are asked to operate on? Doing so involves writing (in my case) Vim code to configure settings for this repo. Since language linting tools invoke compilers and other language runtimes, you need to specify command-line arguments to those tools, and in general, that's not a safe thing you can read from the repository configuration, since just viewing files should not be able to execute arbitrary code[0]. Languages such as Perl which can execute arbitrary code with compile checks have to be enabled explicitly with ALE (which is what I'm using). I need to do a reroll to address some other issues people brought up, so I can remove this line. [0] Pass "-o .bashrc" to the preprocessor, for example. -- brian m. carlson: Houston, Texas, US OpenPGP: https://keybase.io/bk2204 signature.asc Description: PGP signature
Re: [PATCH v4 10/12] Add a base implementation of SHA-256 support
"brian m. carlson" writes: >> > + >> > +#include "git-compat-util.h" >> >> this shouldn't be needed and might be discouraged as per the >> instructions in Documentation/CodingGuidelines > > This may not strictly be needed, but removing it makes the header no > longer self-contained, which blows up my (and others') in-editor > linting. That sounds like bending backwards to please tools, though. Can't these in-editor linting learn the local rules of the codebase they are asked to operate on?
Re: [PATCH v4 10/12] Add a base implementation of SHA-256 support
On Wed, Oct 24, 2018 at 08:02:55PM -0700, Carlo Arenas wrote: > On Wed, Oct 24, 2018 at 7:41 PM brian m. carlson > wrote: > > diff --git a/sha256/block/sha256.h b/sha256/block/sha256.h > > new file mode 100644 > > index 00..38f02f7e6c > > --- /dev/null > > +++ b/sha256/block/sha256.h > > @@ -0,0 +1,26 @@ > > +#ifndef SHA256_BLOCK_SHA256_H > > +#define SHA256_BLOCK_SHA256_H > > + > > +#include "git-compat-util.h" > > this shouldn't be needed and might be discouraged as per the > instructions in Documentation/CodingGuidelines This may not strictly be needed, but removing it makes the header no longer self-contained, which blows up my (and others') in-editor linting. I think it's okay to add this extra header here to keep it self-contained, even if we know that it's not going to be absolutely required. -- brian m. carlson: Houston, Texas, US OpenPGP: https://keybase.io/bk2204 signature.asc Description: PGP signature
Re: [PATCH v4 10/12] Add a base implementation of SHA-256 support
On Thu, Oct 25, 2018 at 4:42 AM brian m. carlson
wrote:
> +static void blk_SHA256_Transform(blk_SHA256_CTX *ctx, const unsigned char
> *buf)
> +{
> +
> + uint32_t S[8], W[64], t0, t1;
> + int i;
> +
> + /* copy state into S */
> + for (i = 0; i < 8; i++) {
> + S[i] = ctx->state[i];
> + }
Maybe remove unnecessary brackets above and below.
> + /* copy the state into 512-bits into W[0..15] */
> + for (i = 0; i < 16; i++, buf += sizeof(uint32_t)) {
> + W[i] = get_be32(buf);
> + }
> +
> + /* fill W[16..63] */
> + for (i = 16; i < 64; i++) {
> + W[i] = gamma1(W[i - 2]) + W[i - 7] + gamma0(W[i - 15]) + W[i
> - 16];
> + }
[...]
> + RND(S[2],S[3],S[4],S[5],S[6],S[7],S[0],S[1],62,0xbef9a3f7);
> + RND(S[1],S[2],S[3],S[4],S[5],S[6],S[7],S[0],63,0xc67178f2);
> +
> +
Spurious new line.
> + for (i = 0; i < 8; i++) {
> + ctx->state[i] = ctx->state[i] + S[i];
> + }
Maybe remove unnecessary brackets and use "+=", like:
for (i = 0; i < 8; i++)
ctx->state[i] += S[i];
Re: [PATCH v4 10/12] Add a base implementation of SHA-256 support
On Wed, Oct 24, 2018 at 7:41 PM brian m. carlson wrote: > diff --git a/sha256/block/sha256.h b/sha256/block/sha256.h > new file mode 100644 > index 00..38f02f7e6c > --- /dev/null > +++ b/sha256/block/sha256.h > @@ -0,0 +1,26 @@ > +#ifndef SHA256_BLOCK_SHA256_H > +#define SHA256_BLOCK_SHA256_H > + > +#include "git-compat-util.h" this shouldn't be needed and might be discouraged as per the instructions in Documentation/CodingGuidelines Carlo
