[PATCH] D16325: Add support for computing SHA1 in LLVM

Mehdi AMINI via llvm-commits llvm-commits at lists.llvm.org
Wed Jan 20 17:30:37 PST 2016


joker.eph added inline comments.

================
Comment at: include/llvm/Support/SHA1.h:38
@@ +37,3 @@
+  /// last call to init()
+  StringRef result();
+
----------------
filcab wrote:
> joker.eph wrote:
> > filcab wrote:
> > > We probably want to tell people to not reuse SHA1 objects, and might want to assert in debug mode.
> > > After all, you could end up hashing weird things like the message from the first call to result() + some padding (and not any bytes you added later).
> > > 
> > > With this, you could even re-use the internal state for the result, but that might not be worth it :-)
> > You mean there is no interest in supporting this:
> > 
> > ```
> > TEST(raw_sha1_ostreamTest, Intermediate) {
> >   llvm::raw_sha1_ostream Sha1Stream;
> >   Sha1Stream << "Hello";
> >   auto Hash = toHex(Sha1Stream.sha1());
> > 
> >   ASSERT_EQ("F7FF9E8B7BB2E09B70935A5D785E0CC5D9D0ABF0", Hash);
> > 
> >   Sha1Stream << " World!";
> >   Hash = toHex(Sha1Stream.sha1());
> > 
> >   ASSERT_EQ("2EF7BDE608CE5404E97D5F042F95F89F1C232871", Hash);
> > }
> > ```
> > 
> > right?
> > Is it because it is potentially allowing misuse?
> As written we don't support that (you'll addUncounted one bit, and then the padding. Afterwards, adding to the stream won't do what it would need to do). I'd like that to be explicit in the documentation (and I'd like an assert or two to make it trigger in debug mode, if that's attempted).
I'm not sure what you mean, the InternalState is saved and restored in `SHA1::result() ` so that we can resume as is. (this unittest is passing)


http://reviews.llvm.org/D16325





More information about the llvm-commits mailing list