* [alsa-devel] [PATCH] ALSA: uapi: Replace __packed with __attribute__((packed)) @ 2020-02-07 16:15 Amadeusz Sławiński 2020-02-07 15:17 ` Takashi Iwai 0 siblings, 1 reply; 4+ messages in thread From: Amadeusz Sławiński @ 2020-02-07 16:15 UTC (permalink / raw) To: Jaroslav Kysela, Takashi Iwai, Guenter Roeck, Mark Brown, alsa-devel Cc: Amadeusz Sławiński 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@linux.intel.com> --- 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@alsa-project.org https://mailman.alsa-project.org/mailman/listinfo/alsa-devel ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [alsa-devel] [PATCH] ALSA: uapi: Replace __packed with __attribute__((packed)) 2020-02-07 16:15 [alsa-devel] [PATCH] ALSA: uapi: Replace __packed with __attribute__((packed)) Amadeusz Sławiński @ 2020-02-07 15:17 ` Takashi Iwai 2020-02-07 16:02 ` Amadeusz Sławiński 0 siblings, 1 reply; 4+ messages in thread From: Takashi Iwai @ 2020-02-07 15:17 UTC (permalink / raw) To: Amadeusz Sławiński Cc: Guenter Roeck, alsa-devel, Mark Brown, Takashi Iwai 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@linux.intel.com> Is it a general consensus that we have to replace that? Userspace doesn't know of __u16 unless it's defined, either, for example. 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@alsa-project.org https://mailman.alsa-project.org/mailman/listinfo/alsa-devel ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [alsa-devel] [PATCH] ALSA: uapi: Replace __packed with __attribute__((packed)) 2020-02-07 15:17 ` Takashi Iwai @ 2020-02-07 16:02 ` Amadeusz Sławiński 2020-02-07 16:11 ` Takashi Iwai 0 siblings, 1 reply; 4+ messages in thread From: Amadeusz Sławiński @ 2020-02-07 16:02 UTC (permalink / raw) To: Takashi Iwai; +Cc: Guenter Roeck, alsa-devel, Mark Brown, Takashi Iwai 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@linux.intel.com> > > 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@alsa-project.org > https://mailman.alsa-project.org/mailman/listinfo/alsa-devel > _______________________________________________ Alsa-devel mailing list Alsa-devel@alsa-project.org https://mailman.alsa-project.org/mailman/listinfo/alsa-devel ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [alsa-devel] [PATCH] ALSA: uapi: Replace __packed with __attribute__((packed)) 2020-02-07 16:02 ` Amadeusz Sławiński @ 2020-02-07 16:11 ` Takashi Iwai 0 siblings, 0 replies; 4+ messages in thread From: Takashi Iwai @ 2020-02-07 16:11 UTC (permalink / raw) To: Amadeusz SX2awiX4ski; +Cc: Guenter Roeck, alsa-devel, Mark Brown, Takashi Iwai On Fri, 07 Feb 2020 17:02:27 +0100, Amadeusz SX2awiX4ski wrote: > > 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@linux.intel.com> > > > > 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. It's a different reason from "userspace doesn't know what __packed is" :) I'm not against the change, but rather wondering about the rationale behind the changes. > 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. This was rather an oversight and we should have converted at copying over the alsa-lib repo. It's not an issue in the kernel header, per se. thanks, Takashi > 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@alsa-project.org > > https://mailman.alsa-project.org/mailman/listinfo/alsa-devel > > > _______________________________________________ Alsa-devel mailing list Alsa-devel@alsa-project.org https://mailman.alsa-project.org/mailman/listinfo/alsa-devel ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2020-02-07 16:12 UTC | newest] Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-02-07 16:15 [alsa-devel] [PATCH] ALSA: uapi: Replace __packed with __attribute__((packed)) Amadeusz Sławiński 2020-02-07 15:17 ` Takashi Iwai 2020-02-07 16:02 ` Amadeusz Sławiński 2020-02-07 16:11 ` Takashi Iwai
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.