Re: [Xen-devel] [PATCH 1/2] Adding support for coverage information

[Frediano Ziglio <frediano.ziglio@xxxxxxxxxx>]

On Tue, 2013-02-05 at 08:38 +0000, Jan Beulich wrote:
> >>> On 04.02.13 at 18:08, Frediano Ziglio <frediano.ziglio@xxxxxxxxxx> wrote:
> > --- a/xen/Rules.mk
> > +++ b/xen/Rules.mk
> > @@ -103,6 +103,11 @@ subdir-all := $(subdir-y) $(subdir-n)
> >  
> >  $(filter %.init.o,$(obj-y) $(obj-bin-y)): CFLAGS += -DINIT_SECTIONS_ONLY
> >  
> > +
> > +ifeq ($(coverage),y)
> > +$(filter-out %.init.o,$(obj-y) $(obj-bin-y)): CFLAGS += -fprofile-arcs 
> > -ftest-coverage -DTEST_COVERAGE
> > +endif
> 
> So you had tried out the simplified version of this that I had
> suggested, it worked, yet you still don't use it?
> 

Yes, I forget to explain why I didn't change this (apart I forgot to
remove the extra blank line).

It seems that if I change this code as you suggested the code compile
correctly but if I extract the informations there are less data.
Now I don't understand if this is just an error or something more
critical.

Currently I'm trying to understand which file change and why.

It seems that these files are compiled with different options

xen/common/libelf/libelf-loader.o
xen/common/libelf/libelf.o
xen/common/libelf/libelf-dominfo.o
xen/common/libelf/built_in.o
xen/common/libelf/libelf-tools.o
xen/common/libelf/libelf-temp.o

I'll try to understand better what's going on.

> > +void init_coverage(void)
> 
> As already said previously: __init!
> 
> > +#ifdef TEST_COVERAGE
> > +void __init init_coverage(void);
> 
> And as also already said previously - no __init on declarations!
> 

Got it, I checked code and understood. First patch the __init was
correctly in the .c file and somebody complaints so I moved to .h (or
probably I misunderstood and the solution was only to remove from
header).

> Please, don't waste reviewers' time by claiming you incorporated
> review comments when you really didn't.
> 
> Jan
> 

Frediano

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel