Re: [Libevent-users] Is evthread_use_pthreads required for isolated evdns event_bases?

[Nick Mathewson <nickm@xxxxxxxxxxxxx>]

On Thu, Jun 20, 2013 at 7:57 PM, Joseph Spadavecchia <joseph@xxxxxxxxxxx> wrote:
> On 19 Jun 2013, at 17:04, Nick Mathewson <nickm@xxxxxxxxxxxxx> wrote:
>
> […]
>
> It might be prudent to have a code review before I go any further; sound
> good?
>
>
> I think it does.  If you put at a branch in a public git repository
> somewhere, that's the best format for me to look at.
>
>
> Here ya go!  https://github.com/jspada/libevent/tree/secure_rng
>
> The changes aren't as minimal as I would prefer, but I'm getting familiar
> with the code and could improve it.  I'd look to reduce the amount of
> change, if you think that's the way to go.

I think smaller commits would be easier to review, yeah.

I was wondering whether it might not make more sense to have the
global structure be the one that gets locked, and the struct variant
be the unlocked one, so that we can avoid the extra lock overhead by
nesting it inside the existing structure locks.

Two minor things that I spotted while skimming:
  * <inttypes.h> isn't standard, and (sadly) <stdint.h> isn't
universal.  We define reasonable replacements in event2/util.h,
though.
  * We don't want to declare our own non-static functions with the
same names as library functions if we can help it; it can get
confusing.  (Also, by convention, if a name is exported by a module
but it isn't a public API of libevent, we suffix it with a _)



> I've run the regress tests on Linux and OSX (223 tests ok and 9 skipped),
> but I've not tested the code more than this.  At this stage I'm looking for
> your feedback on the overall structure, and finer details, so I can get it
> into a form that you're happy with.
>
> Oh, and I've implemented the BSD/OSX compatibility code, so it's all there.
>
> Many thanks,
> *Joe
>
>
***********************************************************************
To unsubscribe, send an e-mail to majordomo@xxxxxxxxxxxxx with
unsubscribe libevent-users    in the body.