| Message ID | 20171026091938.59247-6-roger.pau@citrix.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, Oct 26, 2017 at 10:19:34AM +0100, Roger Pau Monne wrote: > > diff --git a/xen/common/coverage/Makefile b/xen/common/coverage/Makefile > index f68d050eca..0e0510679e 100644 > --- a/xen/common/coverage/Makefile > +++ b/xen/common/coverage/Makefile > @@ -1,4 +1,4 @@ > -obj-y += gcov_base.o gcov.o > +obj-y += gcov_base.o gcov.o coverage.o > obj-$(CONFIG_GCOV_FORMAT_3_4) += gcc_3_4.o > obj-$(CONFIG_GCOV_FORMAT_4_7) += gcc_4_7.o > obj-$(CONFIG_GCOV_FORMAT_4_9) += gcc_4_9.o > diff --git a/xen/common/coverage/coverage.c b/xen/common/coverage/coverage.c > new file mode 100644 > index 0000000000..1dec6944be > --- /dev/null > +++ b/xen/common/coverage/coverage.c > @@ -0,0 +1,71 @@ > +/* > + * Generic functionality for coverage analysis. > + * > + * Copyright (C) 2017 Citrix Systems R&D > + * > + * This program is free software; you can redistribute it and/or > + * modify it under the terms and conditions of the GNU General Public > + * License, version 2, as published by the Free Software Foundation. > + * > + * This program is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > + * General Public License for more details. > + * > + * You should have received a copy of the GNU General Public > + * License along with this program; If not, see <http://www.gnu.org/licenses/>. > + */ > + > +#include <xen/errno.h> > +#include <xen/guest_access.h> > +#include <xen/types.h> > +#include <xen/coverage.h> Please sort this. The rest appears to be pure code motion to me. It looks fine.
On Mon, Oct 30, 2017 at 04:48:21PM +0000, Wei Liu wrote: > On Thu, Oct 26, 2017 at 10:19:34AM +0100, Roger Pau Monne wrote: > > > > diff --git a/xen/common/coverage/Makefile b/xen/common/coverage/Makefile > > index f68d050eca..0e0510679e 100644 > > --- a/xen/common/coverage/Makefile > > +++ b/xen/common/coverage/Makefile > > @@ -1,4 +1,4 @@ > > -obj-y += gcov_base.o gcov.o > > +obj-y += gcov_base.o gcov.o coverage.o > > obj-$(CONFIG_GCOV_FORMAT_3_4) += gcc_3_4.o > > obj-$(CONFIG_GCOV_FORMAT_4_7) += gcc_4_7.o > > obj-$(CONFIG_GCOV_FORMAT_4_9) += gcc_4_9.o > > diff --git a/xen/common/coverage/coverage.c b/xen/common/coverage/coverage.c > > new file mode 100644 > > index 0000000000..1dec6944be > > --- /dev/null > > +++ b/xen/common/coverage/coverage.c > > @@ -0,0 +1,71 @@ > > +/* > > + * Generic functionality for coverage analysis. > > + * > > + * Copyright (C) 2017 Citrix Systems R&D > > + * > > + * This program is free software; you can redistribute it and/or > > + * modify it under the terms and conditions of the GNU General Public > > + * License, version 2, as published by the Free Software Foundation. > > + * > > + * This program is distributed in the hope that it will be useful, > > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > > + * General Public License for more details. > > + * > > + * You should have received a copy of the GNU General Public > > + * License along with this program; If not, see <http://www.gnu.org/licenses/>. > > + */ > > + > > +#include <xen/errno.h> > > +#include <xen/guest_access.h> > > +#include <xen/types.h> > > +#include <xen/coverage.h> > > Please sort this. OK, I will have to include type.h in coverage.h then. Thanks, Roger.
>>> On 30.10.17 at 17:57, <roger.pau@citrix.com> wrote: > On Mon, Oct 30, 2017 at 04:48:21PM +0000, Wei Liu wrote: >> On Thu, Oct 26, 2017 at 10:19:34AM +0100, Roger Pau Monne wrote: >> > --- /dev/null >> > +++ b/xen/common/coverage/coverage.c >> > @@ -0,0 +1,71 @@ >> > +/* >> > + * Generic functionality for coverage analysis. >> > + * >> > + * Copyright (C) 2017 Citrix Systems R&D >> > + * >> > + * This program is free software; you can redistribute it and/or >> > + * modify it under the terms and conditions of the GNU General Public >> > + * License, version 2, as published by the Free Software Foundation. >> > + * >> > + * This program is distributed in the hope that it will be useful, >> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of >> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU >> > + * General Public License for more details. >> > + * >> > + * You should have received a copy of the GNU General Public >> > + * License along with this program; If not, see <http://www.gnu.org/licenses/>. >> > + */ >> > + >> > +#include <xen/errno.h> >> > +#include <xen/guest_access.h> >> > +#include <xen/types.h> >> > +#include <xen/coverage.h> >> >> Please sort this. > > OK, I will have to include type.h in coverage.h then. But that's not something depending on the ordering request: No matter that we have many examples to the contrary, headers (at least non-private ones) shouldn't really rely on other things to have been included up front. Jan
>>> On 26.10.17 at 11:19, <roger.pau@citrix.com> wrote: > --- a/xen/common/coverage/Makefile > +++ b/xen/common/coverage/Makefile > @@ -1,4 +1,4 @@ > -obj-y += gcov_base.o gcov.o > +obj-y += gcov_base.o gcov.o coverage.o These would better be sorted, too. > --- a/xen/include/xen/coverage.h > +++ b/xen/include/xen/coverage.h > @@ -9,6 +9,7 @@ struct cov_sysctl_ops { > void (*reset_counters)(void); > int (*dump)(XEN_GUEST_HANDLE_PARAM(char), uint32_t *); > }; > +extern struct cov_sysctl_ops cov_ops; I don't think this is the right place to put this declaration (and perhaps also the struct one) - it's not part of the outside visible interface of the coverage code, but an internal aspect. Such should live in a private header, unless they need to be exposed to e.g. arch specific code (which doesn't appear to be the case here). Jan
diff --git a/xen/common/coverage/Makefile b/xen/common/coverage/Makefile index f68d050eca..0e0510679e 100644 --- a/xen/common/coverage/Makefile +++ b/xen/common/coverage/Makefile @@ -1,4 +1,4 @@ -obj-y += gcov_base.o gcov.o +obj-y += gcov_base.o gcov.o coverage.o obj-$(CONFIG_GCOV_FORMAT_3_4) += gcc_3_4.o obj-$(CONFIG_GCOV_FORMAT_4_7) += gcc_4_7.o obj-$(CONFIG_GCOV_FORMAT_4_9) += gcc_4_9.o diff --git a/xen/common/coverage/coverage.c b/xen/common/coverage/coverage.c new file mode 100644 index 0000000000..1dec6944be --- /dev/null +++ b/xen/common/coverage/coverage.c @@ -0,0 +1,71 @@ +/* + * Generic functionality for coverage analysis. + * + * Copyright (C) 2017 Citrix Systems R&D + * + * This program is free software; you can redistribute it and/or + * modify it under the terms and conditions of the GNU General Public + * License, version 2, as published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * General Public License for more details. + * + * You should have received a copy of the GNU General Public + * License along with this program; If not, see <http://www.gnu.org/licenses/>. + */ + +#include <xen/errno.h> +#include <xen/guest_access.h> +#include <xen/types.h> +#include <xen/coverage.h> + +#include <public/sysctl.h> + +int sysctl_cov_op(struct xen_sysctl_cov_op *op) +{ + int ret; + + switch ( op->cmd ) + { + case XEN_SYSCTL_COV_get_size: + op->size = cov_ops.get_size(); + ret = 0; + break; + + case XEN_SYSCTL_COV_read: + { + XEN_GUEST_HANDLE_PARAM(char) buf; + uint32_t size = op->size; + + buf = guest_handle_cast(op->buffer, char); + + ret = cov_ops.dump(buf, &size); + op->size = size; + + break; + } + + case XEN_SYSCTL_COV_reset: + cov_ops.reset_counters(); + ret = 0; + break; + + default: + ret = -ENOSYS; + break; + } + + return ret; +} + +/* + * Local variables: + * mode: C + * c-file-style: "BSD" + * c-basic-offset: 4 + * tab-width: 4 + * indent-tabs-mode: nil + * End: + */ diff --git a/xen/common/coverage/gcov.c b/xen/common/coverage/gcov.c index 66c4075f8a..d321cbc3fc 100644 --- a/xen/common/coverage/gcov.c +++ b/xen/common/coverage/gcov.c @@ -21,8 +21,6 @@ #include <xen/types.h> #include <xen/coverage.h> -#include <public/sysctl.h> - #include "gcov.h" /** @@ -210,49 +208,12 @@ static int gcov_dump_all(XEN_GUEST_HANDLE_PARAM(char) buffer, return ret; } -static struct cov_sysctl_ops cov_ops = { +struct cov_sysctl_ops cov_ops = { .get_size = gcov_get_size, .reset_counters = gcov_reset_all_counters, .dump = gcov_dump_all, }; -int sysctl_cov_op(struct xen_sysctl_cov_op *op) -{ - int ret; - - switch ( op->cmd ) - { - case XEN_SYSCTL_COV_get_size: - op->size = cov_ops.get_size(); - ret = 0; - break; - - case XEN_SYSCTL_COV_read: - { - XEN_GUEST_HANDLE_PARAM(char) buf; - uint32_t size = op->size; - - buf = guest_handle_cast(op->buffer, char); - - ret = cov_ops.dump(buf, &size); - op->size = size; - - break; - } - - case XEN_SYSCTL_COV_reset: - cov_ops.reset_counters(); - ret = 0; - break; - - default: - ret = -ENOSYS; - break; - } - - return ret; -} - /* * Local variables: * mode: C diff --git a/xen/include/xen/coverage.h b/xen/include/xen/coverage.h index 9078330109..de400620bf 100644 --- a/xen/include/xen/coverage.h +++ b/xen/include/xen/coverage.h @@ -9,6 +9,7 @@ struct cov_sysctl_ops { void (*reset_counters)(void); int (*dump)(XEN_GUEST_HANDLE_PARAM(char), uint32_t *); }; +extern struct cov_sysctl_ops cov_ops; int sysctl_cov_op(struct xen_sysctl_cov_op *op); #endif
It will contain the generic implementation of sysctl_cov_op, which will be shared between all the coverage implementations. Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> --- Cc: Andrew Cooper <andrew.cooper3@citrix.com> Cc: George Dunlap <George.Dunlap@eu.citrix.com> Cc: Ian Jackson <ian.jackson@eu.citrix.com> Cc: Jan Beulich <jbeulich@suse.com> Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> Cc: Stefano Stabellini <sstabellini@kernel.org> Cc: Tim Deegan <tim@xen.org> Cc: Wei Liu <wei.liu2@citrix.com> --- xen/common/coverage/Makefile | 2 +- xen/common/coverage/coverage.c | 71 ++++++++++++++++++++++++++++++++++++++++++ xen/common/coverage/gcov.c | 41 +----------------------- xen/include/xen/coverage.h | 1 + 4 files changed, 74 insertions(+), 41 deletions(-) create mode 100644 xen/common/coverage/coverage.c