Re: [PATCH] ALSA: uapi: Replace __packed with __attribute__((packed))

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 2/7/2020 4:17 PM, Takashi Iwai wrote:
On Fri, 07 Feb 2020 17:15:33 +0100,
Amadeusz Sławiński wrote:

Userspace doesn't know what __packed is, change it to
__attribute__((packed)), like in the rest of a header.

Fixes: 348f48220b97 (ASoC: topology: Move v4 manifest header data structures to uapi)

Signed-off-by: Amadeusz Sławiński <amadeuszx.slawinski@xxxxxxxxxxxxxxx>

Is it a general consensus that we have to replace that?

Well, it aligns the packed structs definition with the rest of the file and the program I tried compiling built fine after this adjustment.

As I understand, uapi files are usually installed by scripts/headers_install.pl, which takes care of replacing __packed with __attribute((packed)). However if I do 'make install' in alsa-lib it seems to install this one header containing __packed.
In theory alsa-lib also installs /usr/include/alsa/sound/type_compat.h
but to me it seems more like a workaround if I can have working header without additional includes. Also while there is a few more headers in kernel with __packed, this one is the only one included in alsa-lib and being installed by it:

$ pwd
.../alsa-lib
$ grep -RI __packed *
include/sound/type_compat.h:#ifndef __packed
include/sound/type_compat.h:#define __packed __attribute__((packed))
include/sound/uapi/asoc.h:} __packed;
include/sound/uapi/asoc.h:} __packed;
include/sound/uapi/asoc.h:} __packed;
include/sound/uapi/asoc.h:} __packed;
$ pwd
.../linux
$ grep -RI __packed include/uapi/sound
include/uapi/sound/asoc.h:} __packed;
include/uapi/sound/asoc.h:} __packed;
include/uapi/sound/asoc.h:} __packed;
include/uapi/sound/asoc.h:} __packed;
include/uapi/sound/sof/fw.h:} __packed;
include/uapi/sound/sof/fw.h:} __packed;
include/uapi/sound/sof/fw.h:} __packed;
include/uapi/sound/sof/header.h:}  __packed;
include/uapi/sound/skl-tplg-interface.h:} __packed;
include/uapi/sound/skl-tplg-interface.h:} __packed;
include/uapi/sound/skl-tplg-interface.h:} __packed;
include/uapi/sound/skl-tplg-interface.h:} __packed;
include/uapi/sound/skl-tplg-interface.h:} __packed;
include/uapi/sound/skl-tplg-interface.h:} __packed;
$ grep -RI __packed include/uapi/sound | wc -l
14
$ grep -RI __attribute__\(\(packed include/uapi/sound | wc -l
40

I would say it is better to be consistent.

> Userspace doesn't know of __u16 unless it's defined, either, for
> example.

The header itself has <linux/types.h> include which seems to be enough to take care of integer type definitions.

Amadeusz


thanks,

Takashi

---
  include/uapi/sound/asoc.h | 8 ++++----
  1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/include/uapi/sound/asoc.h b/include/uapi/sound/asoc.h
index 6048553c119d..1ae8b06691cb 100644
--- a/include/uapi/sound/asoc.h
+++ b/include/uapi/sound/asoc.h
@@ -586,7 +586,7 @@ struct snd_soc_tplg_manifest_v4 {
  	__le32 pcm_elems;	/* number of PCM elements */
  	__le32 dai_link_elems;	/* number of DAI link elements */
  	struct snd_soc_tplg_private priv;
-} __packed;
+} __attribute((packed));
/* Stream Capabilities v4 */
  struct snd_soc_tplg_stream_caps_v4 {
@@ -604,7 +604,7 @@ struct snd_soc_tplg_stream_caps_v4 {
  	__le32 period_size_max;	/* max period size bytes */
  	__le32 buffer_size_min;	/* min buffer size bytes */
  	__le32 buffer_size_max;	/* max buffer size bytes */
-} __packed;
+} __attribute((packed));
/* PCM v4 */
  struct snd_soc_tplg_pcm_v4 {
@@ -619,7 +619,7 @@ struct snd_soc_tplg_pcm_v4 {
  	struct snd_soc_tplg_stream stream[SND_SOC_TPLG_STREAM_CONFIG_MAX]; /* for DAI link */
  	__le32 num_streams;	/* number of streams */
  	struct snd_soc_tplg_stream_caps_v4 caps[2]; /* playback and capture for DAI */
-} __packed;
+} __attribute((packed));
/* Physical link config v4 */
  struct snd_soc_tplg_link_config_v4 {
@@ -627,6 +627,6 @@ struct snd_soc_tplg_link_config_v4 {
  	__le32 id;              /* unique ID - used to match */
  	struct snd_soc_tplg_stream stream[SND_SOC_TPLG_STREAM_CONFIG_MAX]; /* supported configs playback and captrure */
  	__le32 num_streams;     /* number of streams */
-} __packed;
+} __attribute((packed));
#endif
--
2.17.1

_______________________________________________
Alsa-devel mailing list
Alsa-devel@xxxxxxxxxxxxxxxx
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel

_______________________________________________
Alsa-devel mailing list
Alsa-devel@xxxxxxxxxxxxxxxx
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel




[Index of Archives]     [ALSA User]     [Linux Audio Users]     [Pulse Audio]     [Kernel Archive]     [Asterisk PBX]     [Photo Sharing]     [Linux Sound]     [Video 4 Linux]     [Gimp]     [Yosemite News]

  Powered by Linux