* 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
* [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
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.