Re: [Xen-devel] [RFC PATCH 4/8] gcov: add new interface and 3.4 and 4.7 format support

[Wei Liu <wei.liu2@xxxxxxxxxx>]

On Fri, Oct 07, 2016 at 05:52:50AM -0600, Jan Beulich wrote:
> >>> On 07.10.16 at 13:43, <wei.liu2@xxxxxxxxxx> wrote:
> > On Fri, Oct 07, 2016 at 12:10:25PM +0100, Wei Liu wrote:
> >> On Fri, Oct 07, 2016 at 05:06:42AM -0600, Jan Beulich wrote:
> >> > >>> On 06.10.16 at 16:37, <wei.liu2@xxxxxxxxxx> wrote:
> >> > > A new sysctl interface for passing gcov data back to userspace. The new
> >> > > interface uses a customised record file format. The new sysctl reuses
> >> > > original sysctl number but renames the op to gcov_op.
> >> > > 
> >> > > Both gcc 3.4 and 4.7 format are supported. The code is rewritten so 
> >> > > that
> >> > > a new format can be easily added in the future.  Version specific code
> >> > > is grouped into different files. The format one needs to use can be
> >> > > picked via Kconfig. The default format is 4.7 format.
> >> > > 
> >> > > Userspace programs to handle extracted data will come in a later patch.
> >> > > 
> >> > > Signed-off-by: Wei Liu <wei.liu2@xxxxxxxxxx>
> >> > 
> >> > Looks reasonable, but wants some cosmetics done: Constification
> >> > seems to b possible in a number of places, labels should be
> >> > indented by at least one blank, and pointless initializers would be
> >> > nice to see eliminated. Also, unless at least the removal patch is
> >> > still meant to go in for 4.8, you'll need to bump
> >> > XEN_SYSCTL_INTERFACE_VERSION in the patch here.
> >> > 
> >> 
> >> Regarding cosmetic issues, I will try to fix as many as possible.
> >> 
> >> The removal patch won't go in 4.8 because though the implementation has
> >> a lot of limitations it might still be useful to some people. I will
> >> bump XEN_SYSCTL_INTERFACE_VERSION here in this patch.
> >> 
> > 
> > I go through most (if not all) initialisers, actually they are necessary:
> > 
> > 1. those accumulative counters need to be initialised to 0.
> 
> In e.g.
> 
> +    uint32_t total_size = 0;
> +    struct gcov_info *info = NULL;
> +
> +    /* Magic number XCOV */
> +    total_size += sizeof(uint32_t);
> 
> I can't see why the sizeof(uint32_t) can't be the initializer right away,
> or the initializer be dropped and = used in place of += .
> 

Sure, I can change this and check if there are other similar instances.

Wei.

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