[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Xen-devel] [PATCH] xen/coverage: wrap coverage related things under 'CONFIG_COVERAGE'
- To: Jan Beulich <JBeulich@xxxxxxxx>
- From: chenbaodong <chenbaodong@xxxxxxxxxx>
- Date: Wed, 12 Jun 2019 08:23:34 +0800
- Cc: Stefano Stabellini <sstabellini@xxxxxxxxxx>, WeiLiu <wl@xxxxxxx>, Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>, George Dunlap <George.Dunlap@xxxxxxxxxxxxx>, Andrew Cooper <andrew.cooper3@xxxxxxxxxx>, Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx>, Tim Deegan <tim@xxxxxxx>, Julien Grall <julien.grall@xxxxxxx>, xen-devel <xen-devel@xxxxxxxxxxxxxxxxxxxx>, Roger Pau Monne <roger.pau@xxxxxxxxxx>
- Delivery-date: Wed, 12 Jun 2019 00:23:52 +0000
- List-id: Xen developer discussion <xen-devel.lists.xenproject.org>
On 6/11/19 22:03, Jan Beulich wrote:
On 11.06.19 at 08:02, <chenbaodong@xxxxxxxxxx> wrote:
--- a/xen/arch/x86/xen.lds.S
+++ b/xen/arch/x86/xen.lds.S
@@ -240,12 +240,14 @@ SECTIONS
*(.altinstructions)
__alt_instructions_end = .;
+#if defined(CONFIG_COVERAGE)
. = ALIGN(8);
__ctors_start = .;
*(.ctors)
*(.init_array)
*(SORT(.init_array.*))
__ctors_end = .;
+#endif
How is this (only) coverage related? And how is making this conditional
going to help in any way?
Hello Jan,
When i read the code 'init_constructors()', i want to understand when
it's used.
I can not find any helper macros like '__init' in init.h, put things in
this section.
Also run under arm foundation platform, the section is empty.
So i check commit history and found it's commit logs: it is coverage
related.
And compiled with CONFIG_COVERAGE enabled, this section is not empty
anymore.
So the patch mainly want to clarify the code is coverage related,
which want to help newcomer easily understand this code.
Am i misunderstanding here?
And if we were to take this, then please use the shorter #ifdef.
Yes, can be fixed.
--- a/xen/common/lib.c
+++ b/xen/common/lib.c
@@ -491,15 +491,20 @@ unsigned long long parse_size_and_unit(const char *s,
const char **ps)
return ret;
}
+#if defined(CONFIG_COVERAGE)
typedef void (*ctor_func_t)(void);
extern const ctor_func_t __ctors_start[], __ctors_end[];
+#endif
Again - how does this help?
Want to clarify this is coverage related code.
+/* see 'docs/hypervisor-guide/code-coverage.rst' */
void __init init_constructors(void)
There's no mention of this function in the referenced docs file.
Same as above.
{
+#if defined(CONFIG_COVERAGE)
const ctor_func_t *f;
for ( f = __ctors_start; f < __ctors_end; ++f )
(*f)();
+#endif
/* Putting this here seems as good (or bad) as any other place. */
Again, besides lacking suitable reasoning you also should look
more closely, in this case where exactly it makes sense to place
the #endif.
The blank line here? If yes, can be removed. i missed this.
Jan
.
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel
|
