| Message ID | 1314621249-958-2-git-send-email-tixy@yxit.co.uk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, 29 Aug 2011, Tixy wrote: > From: Jon Medhurst <tixy@yxit.co.uk> > > Signed-off-by: Jon Medhurst <tixy@yxit.co.uk> > --- > arch/arm/Kconfig.debug | 10 ++++++++++ > 1 files changed, 10 insertions(+), 0 deletions(-) > > diff --git a/arch/arm/Kconfig.debug b/arch/arm/Kconfig.debug > index 81cbe40..8f95417 100644 > --- a/arch/arm/Kconfig.debug > +++ b/arch/arm/Kconfig.debug > @@ -129,4 +129,14 @@ config DEBUG_S3C_UART > The uncompressor code port configuration is now handled > by CONFIG_S3C_LOWLEVEL_UART_PORT. > > +config ARM_KPROBES_TESTS > + bool > + > +config ARM_KPROBES_TEST_MODULE > + tristate "Kprobes test module" > + depends on KPROBES && MODULES > + select ARM_KPROBES_TESTS > + help > + "Perform tests of kprobes API and instruction set simulation" > + Why do you need two config symbols here? Isn't ARM_KPROBES_TEST_MODULE redundant? Nicolas
On Mon, 2011-08-29 at 14:44 -0400, Nicolas Pitre wrote: > On Mon, 29 Aug 2011, Tixy wrote: > > > From: Jon Medhurst <tixy@yxit.co.uk> > > > > Signed-off-by: Jon Medhurst <tixy@yxit.co.uk> > > --- > > arch/arm/Kconfig.debug | 10 ++++++++++ > > 1 files changed, 10 insertions(+), 0 deletions(-) > > > > diff --git a/arch/arm/Kconfig.debug b/arch/arm/Kconfig.debug > > index 81cbe40..8f95417 100644 > > --- a/arch/arm/Kconfig.debug > > +++ b/arch/arm/Kconfig.debug > > @@ -129,4 +129,14 @@ config DEBUG_S3C_UART > > The uncompressor code port configuration is now handled > > by CONFIG_S3C_LOWLEVEL_UART_PORT. > > > > +config ARM_KPROBES_TESTS > > + bool > > + > > +config ARM_KPROBES_TEST_MODULE > > + tristate "Kprobes test module" > > + depends on KPROBES && MODULES > > + select ARM_KPROBES_TESTS > > + help > > + "Perform tests of kprobes API and instruction set simulation" > > + > > Why do you need two config symbols here? Isn't ARM_KPROBES_TEST_MODULE > redundant? When ARM_KPROBES_TEST_MODULE is configured for a stand-alone module, rather than built-in, then #ifdef CONFIG_ARM_KPROBES_TEST_MODULE is false. I found other examples where people seemed to have gotten around this by selecting a second config symbol and copied that. E.g. FTRACE_STARTUP_TEST selects FTRACE_SELFTEST which is then used in #ifdef statements. Is there a better way? If not, I should explain this anyway in the changelog.
On Tue, 30 Aug 2011, Tixy wrote: > On Mon, 2011-08-29 at 14:44 -0400, Nicolas Pitre wrote: > > On Mon, 29 Aug 2011, Tixy wrote: > > > > > From: Jon Medhurst <tixy@yxit.co.uk> > > > > > > Signed-off-by: Jon Medhurst <tixy@yxit.co.uk> > > > --- > > > arch/arm/Kconfig.debug | 10 ++++++++++ > > > 1 files changed, 10 insertions(+), 0 deletions(-) > > > > > > diff --git a/arch/arm/Kconfig.debug b/arch/arm/Kconfig.debug > > > index 81cbe40..8f95417 100644 > > > --- a/arch/arm/Kconfig.debug > > > +++ b/arch/arm/Kconfig.debug > > > @@ -129,4 +129,14 @@ config DEBUG_S3C_UART > > > The uncompressor code port configuration is now handled > > > by CONFIG_S3C_LOWLEVEL_UART_PORT. > > > > > > +config ARM_KPROBES_TESTS > > > + bool > > > + > > > +config ARM_KPROBES_TEST_MODULE > > > + tristate "Kprobes test module" > > > + depends on KPROBES && MODULES > > > + select ARM_KPROBES_TESTS > > > + help > > > + "Perform tests of kprobes API and instruction set simulation" > > > + > > > > Why do you need two config symbols here? Isn't ARM_KPROBES_TEST_MODULE > > redundant? > > When ARM_KPROBES_TEST_MODULE is configured for a stand-alone module, > rather than built-in, then > > #ifdef CONFIG_ARM_KPROBES_TEST_MODULE > > is false. OK... It seems that, when you have a tristate config symbol FOOBAR, if CONFIG_FOOBAR=y then the CONFIG_FOOBAR preprocessor symbol is defined. If you have CONFIG_FOOBAR=m then the CONFIG_FOOBAR_MODULE preprocessor symbol is defined. In your example above you would end up with CONFIG_ARM_KPROBES_TEST_MODULE_MODULE being defined. > I found other examples where people seemed to have gotten around this > by selecting a second config symbol and copied that. E.g. > FTRACE_STARTUP_TEST selects FTRACE_SELFTEST which is then used in > #ifdef statements. > > Is there a better way? If not, I should explain this anyway in the > changelog. What I'd suggest is that you have: config ARM_KPROBES_TEST tristate ... And then you may use this in the code: #ifdef CONFIG_ARM_KPROBES_TEST_MODULE EXPORT_SYMBOL_GPL(...); #endif so those symbols are only exported when necessary. Nicolas
On Wed, 2011-08-31 at 18:47 -0400, Nicolas Pitre wrote: > On Tue, 30 Aug 2011, Tixy wrote: [...] > > When ARM_KPROBES_TEST_MODULE is configured for a stand-alone module, > > rather than built-in, then > > > > #ifdef CONFIG_ARM_KPROBES_TEST_MODULE > > > > is false. > > OK... It seems that, when you have a tristate config symbol FOOBAR, if > CONFIG_FOOBAR=y then the CONFIG_FOOBAR preprocessor symbol is defined. > If you have CONFIG_FOOBAR=m then the CONFIG_FOOBAR_MODULE preprocessor > symbol is defined. That's useful to know. > In your example above you would end up with > CONFIG_ARM_KPROBES_TEST_MODULE_MODULE being defined. > > > I found other examples where people seemed to have gotten around this > > by selecting a second config symbol and copied that. E.g. > > FTRACE_STARTUP_TEST selects FTRACE_SELFTEST which is then used in > > #ifdef statements. > > > > Is there a better way? If not, I should explain this anyway in the > > changelog. > > What I'd suggest is that you have: > > config ARM_KPROBES_TEST > tristate ... > > And then you may use this in the code: > > #ifdef CONFIG_ARM_KPROBES_TEST_MODULE > EXPORT_SYMBOL_GPL(...); > #endif > > so those symbols are only exported when necessary. Yep, that works and is a lot cleaner, thanks. I've also made the extern declarations for these exported symbols unconditional in kprobes-test.h. It didn't seem worth trying to #ifdef them to only exist when the test code is configured (like I was trying to do in "[PATCH 06/10] ARM: kprobes: Add exports for test code").
diff --git a/arch/arm/Kconfig.debug b/arch/arm/Kconfig.debug index 81cbe40..8f95417 100644 --- a/arch/arm/Kconfig.debug +++ b/arch/arm/Kconfig.debug @@ -129,4 +129,14 @@ config DEBUG_S3C_UART The uncompressor code port configuration is now handled by CONFIG_S3C_LOWLEVEL_UART_PORT. +config ARM_KPROBES_TESTS + bool + +config ARM_KPROBES_TEST_MODULE + tristate "Kprobes test module" + depends on KPROBES && MODULES + select ARM_KPROBES_TESTS + help + "Perform tests of kprobes API and instruction set simulation" + endmenu