Re: [ovs-dev] [PATCH] coverage: Make thread-safe.

2013-08-20 Thread Alex Wang
Thanks for the insight. On Tue, Aug 20, 2013 at 3:39 PM, Ben Pfaff wrote: > To expand on that, usually one defines a non-static variable, in a .c > file, only when an "extern" declaration of the same variable is > already visible from #including an .h file. A definition without a > previous de

Re: [ovs-dev] [PATCH] coverage: Make thread-safe.

2013-08-20 Thread Ben Pfaff
To expand on that, usually one defines a non-static variable, in a .c file, only when an "extern" declaration of the same variable is already visible from #including an .h file. A definition without a previous declaration usually means that the .h file was forgotten. On Tue, Aug 20, 2013 at 02:48

Re: [ovs-dev] [PATCH] coverage: Make thread-safe.

2013-08-20 Thread Alex Wang
I found the answer from this thread: http://openvswitch.org/pipermail/dev/2011-May/008608.html On Tue, Aug 20, 2013 at 2:02 PM, Alex Wang wrote: > May I ask, what rule is this? > > Should we always declare global variable before defining it? or just when > it is enclosed by macros? > > Also, w

Re: [ovs-dev] [PATCH] coverage: Make thread-safe.

2013-08-20 Thread Alex Wang
May I ask, what rule is this? Should we always declare global variable before defining it? or just when it is enclosed by macros? Also, when the "!USE_LINKER_SECTIONS", we should modify the coverage.c the same way. I'll send out a patch, On Tue, Aug 20, 2013 at 1:44 PM, Ben Pfaff wrote: > On

Re: [ovs-dev] [PATCH] coverage: Make thread-safe.

2013-08-20 Thread Ben Pfaff
On Mon, Aug 19, 2013 at 08:08:37AM -0700, Alex Wang wrote: > This looks good to me, > > The only issue may be that some comments need to be changed: > > -/* Sorts coverage counters in descending order by count, within equal > counts > - * alphabetically by name. */ > +/* Sorts coverage counters i

Re: [ovs-dev] [PATCH] coverage: Make thread-safe.

2013-08-20 Thread Ben Pfaff
On Mon, Aug 19, 2013 at 12:32:06PM -0700, Alex Wang wrote: > Just noticed, when compiling with sparse, it issues the warnings like: > > """ > lib/netdev-linux.c:76:1: warning: symbol 'counter_netdev_set_policing' was > not declared. Should it be static? > lib/netdev-linux.c:77:1: warning: symbol '

Re: [ovs-dev] [PATCH] coverage: Make thread-safe.

2013-08-19 Thread Alex Wang
Hey Ben, Just noticed, when compiling with sparse, it issues the warnings like: """ lib/netdev-linux.c:76:1: warning: symbol 'counter_netdev_set_policing' was not declared. Should it be static? lib/netdev-linux.c:77:1: warning: symbol 'counter_netdev_arp_lookup' was not declared. Should it be sta

Re: [ovs-dev] [PATCH] coverage: Make thread-safe.

2013-08-19 Thread Alex Wang
This looks good to me, The only issue may be that some comments need to be changed: -/* Sorts coverage counters in descending order by count, within equal counts - * alphabetically by name. */ +/* Sorts coverage counters in descending order by total count, + * within equal total counts alphabetic