On Sep 10 2016 22:41, Takashi Iwai wrote:
On Sat, 10 Sep 2016 09:25:31 +0200, Takashi Sakamoto wrote:
On Sep 10 2016 15:44, Takashi Iwai wrote:
On Sat, 10 Sep 2016 06:50:14 +0200, Takashi Sakamoto wrote:
Hi,
Currently, TLV related protocol is not shared to user land. This is not good in a point of application interfaces, because application developers can't realize the protocol just to see UAPI headers.
For this purpose, this patchset moves all of macros related to TLV to UAPI header. As a result, a header just for kernel land is obsoleted. When adding new items to the protocol, it's added to the UAPI header. This change affects some drivers in media subsystem.
In my concern, this change can break applications. When these macros are already defined in application side and they includes tlv UAPI header directly, 'redefined' warning is generated at preprocess time. But the compilation will be success itself. If these two macros have different content, the result of preprocess is dominated to the order to define. However, the most applications are assumed to use TLV feature via libraries such as alsa-lib, thus I'm optimistic to this concern.
As another my concern, the name of these macros are quite simple, as 'TLV_XXX'. It might be help application developers to rename them with a prefix, as 'SNDRV_CTL_TLV_XXX'. (But not yet. I'm a lazy guy.)
The second patch does simply wrong. You must not obsolete include/sound/tlv.h. Even if it includes only uapi/*, it should be still there.
Any reasons?
The concept and the design.
Don't need to change the root inclusion, it's just to provide cleaner uapi header files, and not meant to be included directly -- it was the basic idea when uapi split was introduced.
OK. I can see what you indicated in this post for UAPI idea[0]. I'm ready to drop the second patch.
Well, how do you think about my concern of macro prefix? For example, we can apply below step:
Put substantial macros with renaming to 'include/uapi.sound/tlv.h': #define SNDRV_CTL_TLV_DATA_LENGTH(...) \ ((unsigned int)sizeof((const unsigned int[]) { __VA_ARGS__ }))
Then, put alias macros to 'include/sound/tlv.h': #include <uapi/sound/tlv.h> #define TLV_LENGTH SNDRV_CTL_TLV_DATA_LENGTH ...
Finally, applications can expand these macro with apparent names with prefix of 'SNDRV_CTL_TLV_DATA_XXX'. I think the prefix prevent application codes from name conflict by including 'uapi/sound/tlv.h'.
Thanks
[0] [PATCH 00/13] UAPI header file split https://lkml.org/lkml/2012/7/20/406
Takashi Sakamoto