Wireshark-bugs: [Wireshark-bugs] [Bug 7680] Added L2TPv3 control message authentication checking
Date Index · Thread Index · Other Months · All Mailing Lists
Date Prev · Date Next · Thread Prev · Thread Next
From: bugzilla-daemon@xxxxxxxxxxxxx
Date: Thu, 30 Aug 2012 05:04:10 -0700 (PDT)
https://bugs.wireshark.org/bugzilla/show_bug.cgi?id=7680

--- Comment #9 from Chris Elston <celston@xxxxxxxxxxx> 2012-08-30 05:04:10 PDT ---
Thanks for the review, Jakub.

(In reply to comment #8)
> Quick review:
> 
> (In reply to comment #3)
> > Created attachment 9030 [details]
> > Split sha1_hmac for incremental use
> 
> For consistency: s/sha1_hmac_starts/sha1_hmac_init/g ?

This is internally consistent within sha1.c, which already has:

  sha1_starts, sha1_update and sha1_finish

so I went with:

  sha1_hmac_starts, sha1_hmac_update and sha1_hmac_finish.

Whereas md5.c has:

  md5_init, md5_append and md5_finish

and therefore I went with:

  md5_hmac_init, md5_hmac_append and md5_hmac_finish

So the choice is which pattern we'd like to be consistent with :)

> (In reply to comment #5)
> > Created attachment 9032 [details]
> > Add L2TPv3 control message authentication check
> 
> se_alloc() never returns NULL.

I didn't know that, thanks. I'll amend accordingly.

> Also I don't see much sense for gotos in this patch, could you remove it?

ack.

-- 
Configure bugmail: https://bugs.wireshark.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching all bug changes.