Nick Mathewson wrote: > On Fri, Oct 20, 2006 at 08:32:06AM -0400, Watson Ladd wrote: >> I looked at HT.h and found some macros that should be functions as they >> use their argument multiple times. > > Hi, Watson, and thanks for your questions! > > I assume you mean HT_INIT, _HT_BUCKET, _HT_SET_HASH, and HT_FOREACH. > (HT_PROTOTYPE and HT_GENERATE are harmless, since they are used to > generate nice safe functions.) > > There are two schools of thought about C macros. One says that you > should make your macros safe even when their arguments have > side-effects. The other says that you should write your macro names > distinctly (say, in all-caps) so that callers can remember not to pass > them arguments with side-effects. I don't think either is the One > True Way; I like making things safer, but I don't have a religious > issue here. > > Now, _HT_BUCKET and _HT_SET_HASH are safe, since they're only used > internally to ht.h, which uses them correctly. [I'd rather not > downgrade them to functions, since they're very much critical-path, > and I don't trust .] > > HT_FOREACH *has* to be a macro, since it introduces syntax. > > HT_INIT is only called in 4 places, none of which misuse it. It's not > a terribly error-prone API, too, so left to my own devices I'd leave > it alone. Still, switching that to a function (say, another inline in > HT_PROTOTYPE()) couldn't hurt; feel free to submit a patch. > >> What are the bad effects of >> conversion? Also, why are certificates rotated in main.c? > > Nearly *everything* that gets launched on a regular schedule gets > launched from main.c. Eventually, we'd like to switch to using > libevent's clever evtimer_set() feature to do our scheduling, but the > current (unclever) implementation is not in the critical performance > path, and it works just fine, so replacing it isn't on our TODO. > > hope this answers your questions, I have attached an untested patch that puts HT_INIT into the HT_PROTOTYPE macro and adds a macro to make HT_INIT refer to the function. -- They who would give up essential Liberty to purchase a little temporary Safety, deserve neither Liberty or Safety --Benjamin Franklin
29,35c29
< #define HT_INIT(root) do { \
< (root)->hth_table_length = 0; \
< (root)->hth_table = NULL; \
< (root)->hth_n_entries = 0; \
< (root)->hth_load_limit = 0; \
< (root)->hth_prime_idx = -1; \
< } while (0)
---
>
62c56
<
---
> #define HT_INIT(name, head)
103c97,105
< int _##name##_HT_REP_OK(struct name *ht); \
---
> int _##name##_HT_REP_OK(struct name *ht); \
> void _##name##_HT_INIT(HT_HEAD(name, type) root){ \
> (root)->hth_table_length = 0; \
> (root)->hth_table = NULL; \
> (root)->hth_n_entries = 0; \
> (root)->hth_load_limit = 0; \
> (root)->hth_prime_idx = -1; \
> }\
> \
Attachment:
signature.asc
Description: OpenPGP digital signature