I found answer from here: http://stackoverflow.com/questions/337449/how-does-one-declare-an-array-of-constant-function-pointers-in-c
so if i understand it correctly: extern int (*build_assert(void))[1]; declare an extern function pointer 'build_assert', pointing to a function that takes no input argument and returns int[1]. Right? But I'm still not clear why we must use (*build_assert(void))? I tried to change it to normal variable declaration like below: /* Build-time assertion for use in a declaration context. */ #define BUILD_ASSERT_DECL(EXPR) \ /* Build-time assertion for use in a declaration context. */ #define BUILD_ASSERT_DECL(EXPR) \ - extern int (*build_assert(void))[BUILD_ASSERT__(EXPR)] + extern int build_assert[BUILD_ASSERT__(EXPR)] But I got "unused variable" warnings. So, is using "(*build_assert(void))" for suppressing the "unused variable" warnings only? Kind Regards, Alex Wang, On Mon, Aug 26, 2013 at 2:39 PM, Alex Wang <[email protected]> wrote: > Also, in viewing the util.h code, I saw, > > extern int (*build_assert(void))[1]; > > want to ask: what is the "(*build_assert(void))" ? > > Thanks, > > > On Mon, Aug 26, 2013 at 1:24 PM, Alex Wang <[email protected]> wrote: > >> Thanks Ben for the review, >> >> >> > +static long long int coverage_run_time = LLONG_MIN; >>> > + >>> > +/* Defines the moving average array index variables. */ >>> > +static unsigned int min_idx = 0; >>> > +static unsigned int hr_idx = 0; >>> >>> It looks to me that min_idx and hr_idx are used only in >>> coverage_run(). You might consider moving them into that function. >>> >>> >> >> Makes sense, I'll adjust this. >> >> >> >>> > +/* Index counter used to compute the moving average array's index. */ >>> > +static unsigned int idx_count = 0; >>> >>> It looks to me like idx_count should be protected by coverage_mutex. >> >> >> >> I'm a bit confused here. Want to ask, that there will only be one thread >> going through the loop in ovs-vswitchd.c (which calls coverage_run()), >> right? If so, idx_count will only be accessed by that thread. Then do we >> still need to protect it? >> >> >> >> >>> > + coverage_array_sum(c[i]->min, MIN_AVG_LEN), >>> > + coverage_array_sum(c[i]->hr, HR_AVG_LEN), >>> > + totals[i])); >>> > + >>> >>> The indentation and parenthesization is not quite right here: >>> ((double) c[i]->min[(idx_count - 1) % >>> MIN_AVG_LEN]) >>> * 1000 / COVERAGE_RUN_INTERVAL, >>> It should be: >>> ((double) c[i]->min[(idx_count - 1) % >>> MIN_AVG_LEN] >>> * 1000 / COVERAGE_RUN_INTERVAL), >>> (I think that CodingStyle implies this--I intended that it should--but >>> it might require a close reading.) >>> >>> Also I might just use 1000.0 instead of a cast to double, because I am >>> not fond of casts: >>> (c[i]->min[(idx_count - 1) % MIN_AVG_LEN] >>> * 1000.0 / COVERAGE_RUN_INTERVAL), >>> >>> >> >> Thanks for pointing these out. I'll fix accordingly. >> >> >> >>> When I can, I like to use build assertions to enforce build-time >>> invariants in place of or in addition to comments. (This is level 9 >>> on Rusty Russell's "Hard To Misuse Positive Score List" at >>> http://ozlabs.org/~rusty/index.cgi/tech/2008-03-30.html. Everyone >>> should read this.) >>> >>> In this case, I think that you can use BUILD_ASSERT_DECL. >>> >> >> Thanks for sharing this reference! >> > >
_______________________________________________ dev mailing list [email protected] http://openvswitch.org/mailman/listinfo/dev
