On Fri, Jun 21, 2013 at 11:32 AM, Joseph Spadavecchia <[email protected]> wrote: > On 21 Jun 2013, at 15:23, Nick Mathewson <[email protected]> wrote: > […] >> 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. > > Yeah, that makes more sense. So, the lock for an evutil_secure_rng would be > that of its parent evdns_base, thus eliminating the need for locks on > individual evutil_secure_rng contexts?
That's what I was thinking, yeah. > Though it does mean that multiple threads cannot use a standalone > evutil_secure_rng without providing their own locking, but that functionality > is probably not needed. Right. > So, is it safe to simply remove the locks from the evutil_secure_rng contexts > and rely on those of their evedns_bases, or do some changes need to be made > at the evedns_base level? We should look at the code to be certain that it's only invoked when we're holding the lock. >> 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. > > Thanks! > >> * 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. > > To be honest, I wasn't really happy with the _r convention I adopted for the > new context-based evutil_secure_rng_get_bytes, etc, functions, but I didn't > want to get bogged down in mnemonics. > > Do you think evutil_secure_rng_get_bytes_r, etc, should be part of a the > external interface, or just internal? If external, what kind of name would > you give them? I think internal-only is fine. Maybe ending them with _nolock_ or _ctx_ or something would be the right approach? >> (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 _) > > OK, so you'd like arc4random_buf_r to become arc4random_buf_, for example? Well, for that one, I don't want to export it at all if we can possibly help it. If we do, then we would need to rename it, since we'd hit another naming convention: we try to make all of our new exported identifiers start with something like "ev" (or "bufferevent" as a legacy exception) to avoid namespace conflicts. yrs, -- Nick *********************************************************************** To unsubscribe, send an e-mail to [email protected] with unsubscribe libevent-users in the body.
