[PATCH 41/45] include/uapi/sound/emu10k1.h: hide gpr_valid, tram_valid and code_valid in userspace

The DECLARE_BITMAP macro is not available in userspace headers.
Fixes userspace compile error:
error: expected specifier-qualifier-list before ‘DECLARE_BITMAP’

Signed-off-by: Mikko Rapeli <redacted>
---
 include/uapi/sound/emu10k1.h | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/include/uapi/sound/emu10k1.h b/include/uapi/sound/emu10k1.h
index ec1535b..f2fd870 100644
--- a/include/uapi/sound/emu10k1.h
+++ b/include/uapi/sound/emu10k1.h

@@ -300,7 +300,9 @@ struct snd_emu10k1_fx8010_control_old_gpr {
 struct snd_emu10k1_fx8010_code {
 	char name[128];
 
+#ifdef __KERNEL__
 	DECLARE_BITMAP(gpr_valid, 0x200); /* bitmask of valid initializers */
+#endif
 	__u32 __user *gpr_map;		/* initializers */
 
 	unsigned int gpr_add_control_count; /* count of GPR controls to add/replace */

@@ -313,11 +315,15 @@ struct snd_emu10k1_fx8010_code {
 	unsigned int gpr_list_control_total; /* total count of GPR controls */
 	struct snd_emu10k1_fx8010_control_gpr __user *gpr_list_controls; /* listed GPR controls */
 
+#ifdef __KERNEL__
 	DECLARE_BITMAP(tram_valid, 0x100); /* bitmask of valid initializers */
+#endif
 	__u32 __user *tram_data_map;	  /* data initializers */
 	__u32 __user *tram_addr_map;	  /* map initializers */
 
+#ifdef __KERNEL__
 	DECLARE_BITMAP(code_valid, 1024); /* bitmask of valid instructions */
+#endif
 	__u32 __user *code;		  /* one instruction - 64 bits */
 };
 

-- 
2.1.4

At Tue, 17 Feb 2015 00:05:44 +0100,
Mikko Rapeli wrote:

The DECLARE_BITMAP macro is not available in userspace headers.
Fixes userspace compile error:
error: expected specifier-qualifier-list before ‘DECLARE_BITMAP’

It's nonsense.  This results in an incompatible structure, thus ABI
would be broken completely (actually this will break the compile of
ld10k1).


Takashi

On Tue, Feb 17, 2015 at 07:27:38AM +0100, Takashi Iwai wrote:

At Tue, 17 Feb 2015 00:05:44 +0100,
Mikko Rapeli wrote:

quoted

The DECLARE_BITMAP macro is not available in userspace headers.
Fixes userspace compile error:
error: expected specifier-qualifier-list before ‘DECLARE_BITMAP’

It's nonsense.  This results in an incompatible structure, thus ABI
would be broken completely (actually this will break the compile of
ld10k1).

None of the exported headers after 'make headers_install' have definition
of DECLARE_BITMAP macro. It is defined in include/linux/types.h which is
different from include/uapi/linux/types.h and missing this definition and
a few other things.

One option would be add DECLARE_BITMAP macro to include/uapi/linux/types.h
and add include/linux/bitops.h to uapi.

Thoughts?

-Mikko

At Wed, 11 Mar 2015 03:22:04 +0200,
Mikko Rapeli wrote:

On Tue, Feb 17, 2015 at 07:27:38AM +0100, Takashi Iwai wrote:

quoted

At Tue, 17 Feb 2015 00:05:44 +0100,
Mikko Rapeli wrote:

quoted

The DECLARE_BITMAP macro is not available in userspace headers.
Fixes userspace compile error:
error: expected specifier-qualifier-list before ‘DECLARE_BITMAP’

It's nonsense.  This results in an incompatible structure, thus ABI
would be broken completely (actually this will break the compile of
ld10k1).

None of the exported headers after 'make headers_install' have definition
of DECLARE_BITMAP macro. It is defined in include/linux/types.h which is
different from include/uapi/linux/types.h and missing this definition and
a few other things.

One option would be add DECLARE_BITMAP macro to include/uapi/linux/types.h
and add include/linux/bitops.h to uapi.

Thoughts?

Are there any other headers like that?  If this is the only one, leave
it as is.  The only program that reads this are some alsa-tools ones
and they have already own DECLARE_BITMAP() definition.  Adding the
extra definition here will even break the compilation out of sudden.


Takashi

On Wednesday 11 March 2015 07:11:18 Takashi Iwai wrote:

At Wed, 11 Mar 2015 03:22:04 +0200,
Mikko Rapeli wrote:

quoted

On Tue, Feb 17, 2015 at 07:27:38AM +0100, Takashi Iwai wrote:

quoted

At Tue, 17 Feb 2015 00:05:44 +0100,
Mikko Rapeli wrote:

quoted

The DECLARE_BITMAP macro is not available in userspace headers.
Fixes userspace compile error:
error: expected specifier-qualifier-list before ‘DECLARE_BITMAP’

It's nonsense.  This results in an incompatible structure, thus ABI
would be broken completely (actually this will break the compile of
ld10k1).

None of the exported headers after 'make headers_install' have definition
of DECLARE_BITMAP macro. It is defined in include/linux/types.h which is
different from include/uapi/linux/types.h and missing this definition and
a few other things.

One option would be add DECLARE_BITMAP macro to include/uapi/linux/types.h
and add include/linux/bitops.h to uapi.

Thoughts?

Are there any other headers like that?  If this is the only one, leave
it as is.  The only program that reads this are some alsa-tools ones
and they have already own DECLARE_BITMAP() definition.  Adding the
extra definition here will even break the compilation out of sudden.

I think it's a worthy goal to have the header files be compilable
standalone, but I don't think we should make the DECLARE_BITMAP()
macro globally visible in user space, in particular because it will
clash with every instance in which user space has a macro of the
same name.

What we could do here is to add a private copy of the macro to emu10k1.h
under a different name, such as __EMU10K1_DECLARE_BITMAP().

	Arnd

At Wed, 11 Mar 2015 10:46:29 +0100,
Arnd Bergmann wrote:

On Wednesday 11 March 2015 07:11:18 Takashi Iwai wrote:

quoted

At Wed, 11 Mar 2015 03:22:04 +0200,
Mikko Rapeli wrote:

quoted

On Tue, Feb 17, 2015 at 07:27:38AM +0100, Takashi Iwai wrote:

quoted

At Tue, 17 Feb 2015 00:05:44 +0100,
Mikko Rapeli wrote:

quoted

The DECLARE_BITMAP macro is not available in userspace headers.
Fixes userspace compile error:
error: expected specifier-qualifier-list before ‘DECLARE_BITMAP’

It's nonsense.  This results in an incompatible structure, thus ABI
would be broken completely (actually this will break the compile of
ld10k1).

None of the exported headers after 'make headers_install' have definition
of DECLARE_BITMAP macro. It is defined in include/linux/types.h which is
different from include/uapi/linux/types.h and missing this definition and
a few other things.

One option would be add DECLARE_BITMAP macro to include/uapi/linux/types.h
and add include/linux/bitops.h to uapi.

Thoughts?

Are there any other headers like that?  If this is the only one, leave
it as is.  The only program that reads this are some alsa-tools ones
and they have already own DECLARE_BITMAP() definition.  Adding the
extra definition here will even break the compilation out of sudden.

I think it's a worthy goal to have the header files be compilable
standalone,

In general yes, but this case is very minor issue:
- the file in question is for a hardware device-specific data
  definition,
- there are only two programs read this file, both can be built
  properly,
- and the device and the programs are very old, modifying such need
  extra care.

but I don't think we should make the DECLARE_BITMAP()
macro globally visible in user space, in particular because it will
clash with every instance in which user space has a macro of the
same name.

What we could do here is to add a private copy of the macro to emu10k1.h
under a different name, such as __EMU10K1_DECLARE_BITMAP().

Yes, it's a better option.


thanks,

Takashi

On Thursday 12 March 2015 07:11:48 Takashi Iwai wrote:

At Wed, 11 Mar 2015 10:46:29 +0100,
Arnd Bergmann wrote:

quoted

On Wednesday 11 March 2015 07:11:18 Takashi Iwai wrote:

quoted

At Wed, 11 Mar 2015 03:22:04 +0200,

Are there any other headers like that?  If this is the only one, leave
it as is.  The only program that reads this are some alsa-tools ones
and they have already own DECLARE_BITMAP() definition.  Adding the
extra definition here will even break the compilation out of sudden.

I think it's a worthy goal to have the header files be compilable
standalone,

In general yes, but this case is very minor issue:
- the file in question is for a hardware device-specific data
  definition,
- there are only two programs read this file, both can be built
  properly,
- and the device and the programs are very old, modifying such need
  extra care.

Right, we should only do it if the goal is to have all uapi headers
includable standalone. For a particular header file there is very
little benefit as you say, but it would be useful if we can automatically
test for regressions with new or modified headers.

	Arnd

At Thu, 12 Mar 2015 09:45:42 +0100,
Arnd Bergmann wrote:

On Thursday 12 March 2015 07:11:48 Takashi Iwai wrote:

quoted

At Wed, 11 Mar 2015 10:46:29 +0100,
Arnd Bergmann wrote:

quoted

On Wednesday 11 March 2015 07:11:18 Takashi Iwai wrote:

quoted

At Wed, 11 Mar 2015 03:22:04 +0200,

Are there any other headers like that?  If this is the only one, leave
it as is.  The only program that reads this are some alsa-tools ones
and they have already own DECLARE_BITMAP() definition.  Adding the
extra definition here will even break the compilation out of sudden.

I think it's a worthy goal to have the header files be compilable
standalone,

In general yes, but this case is very minor issue:
- the file in question is for a hardware device-specific data
  definition,
- there are only two programs read this file, both can be built
  properly,
- and the device and the programs are very old, modifying such need
  extra care.

Right, we should only do it if the goal is to have all uapi headers
includable standalone. For a particular header file there is very
little benefit as you say, but it would be useful if we can automatically
test for regressions with new or modified headers.

True.  Yet another option would be just move this file back from
include/uapi/sound to include/sound.  Basically no user-space programs
care about this file, as they have already a copy of old header fils
in the package.

But maybe defining __EMU10K1_DECLARE_BITMAP() would be the easiest
solution, I suppose.


thanks,

Takashi