On Wed, Feb 03, 2016 at 05:04:32PM -0500, Russell Bryant wrote:
> On 02/03/2016 04:50 PM, Ben Pfaff wrote:
> > Until now, vlog had a macro VLOG_DEFINE_THIS_MODULE, which expanded using
> > VLOG_DEFINE_MODULE, which expanded using VLOG_DEFINE_MODULE__, and the
> > latter macros didn't have any other users. This commit combines them for
> > clarity.
> >
> > Signed-off-by: Ben Pfaff <b...@ovn.org>
>
> Acked-by: Russell Bryant <russ...@ovn.org>
Thanks.
> A couple of things confused me here so I had to bug you on IRC. :-)
>
> Some comments might be helpful:
>
> > @@ -181,12 +174,23 @@ void vlog_rate_limit(const struct vlog_module *, enum
> > vlog_level,
> > * defines a static variable named THIS_MODULE that points to it, for use
> > with
> > * the convenience macros below. */
> > #define VLOG_DEFINE_THIS_MODULE(MODULE) \
> > - VLOG_DEFINE_MODULE(MODULE); \
>
> /* The extern declaration here makes sparse happy. Otherwise it
> suggests that VLM_##MODULE should be delcared static. We don't
> want to make it static, because this approach will generate a
> linker error on duplicate module names */
The first part of the comment, about sparse, makes sense here. Although
the rest is correct, at this point in the series it's incomplete logic
because there are actual references to some of the VLM_* across
translation units. So the latter part makes the most sense only later
in the series (I thought that was what you were asking about) after we
change "struct vlog_module *THIS_MODULE" into "struct vlog_module
this_module".
For now, I added:
/* This extra "extern" declaration makes sparse happy. */ \
and I'll likely push this in a minute or two.
Thanks again!
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
