All of lore.kernel.org
 help / color / mirror / Atom feed
From: Takashi Iwai <tiwai@suse.de>
To: Arnd Bergmann <arnd@arndb.de>
Cc: ALSA Development Mailing List <alsa-devel@alsa-project.org>,
	Baolin Wang <baolin.wang@linaro.org>,
	y2038 Mailman List <y2038@lists.linaro.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	Takashi Iwai <tiwai@suse.com>, Mark Brown <broonie@kernel.org>,
	Baolin Wang <baolin.wang7@gmail.com>
Subject: Re: [alsa-devel] [PATCH v6 5/8] ALSA: Avoid using timespec for struct snd_rawmidi_status
Date: Tue, 12 Nov 2019 21:26:00 +0100	[thread overview]
Message-ID: <s5hmud07sfb.wl-tiwai@suse.de> (raw)
In-Reply-To: <CAK8P3a0esv0T6ALMXJW40B45Qy5BqVSV9rhSD_sVNUc1T+suEg@mail.gmail.com>

On Tue, 12 Nov 2019 21:04:10 +0100,
Arnd Bergmann wrote:
> 
> On Tue, Nov 12, 2019 at 5:38 PM Takashi Iwai <tiwai@suse.de> wrote:
> > On Tue, 12 Nov 2019 16:16:39 +0100, Arnd Bergmann wrote:
> > > +#ifndef __KERNEL__
> > >  struct snd_rawmidi_status {
> > >       int stream;
> > > +     unsigned char pad1[sizeof(time_t) - sizeof(int)];
> > >       struct timespec tstamp;         /* Timestamp */
> > >       size_t avail;                   /* available bytes */
> > >       size_t xruns;                   /* count of overruns since last status (in bytes) */
> > >       unsigned char reserved[16];     /* reserved for future use */
> > >  };
> >
> > Can we use union instead of padding?  Something like:
> >
> > struct snd_rawmidi_status {
> >         union {
> >                 int stream;
> >                 time_t stream_alignment;
> >         };
> >         struct timespec tstamp;         /* Timestamp */
> >         ....
> 
> I think this would work most of the time, though I don't feel this is more
> readable than the other version.
> 
> More importantly, it requires compiling user applications with GNU extensions
> (--std=gnu89 or higher) or C11, but not C99, so this could be a problem
> for some applications.

OK, fair enough.

> If you feel there is a problem with the padding syntax, how about enclosing
> it in a typedef like:
> 
> typedef struct { char pad[sizeof(time_t) - sizeof(int)]; } __time_pad;
> 
> This typedef could be used in several structures from the other patches
> as well.

Yes, that improves the readability.


thanks,

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

WARNING: multiple messages have this Message-ID (diff)
From: Takashi Iwai <tiwai@suse.de>
To: Arnd Bergmann <arnd@arndb.de>
Cc: ALSA Development Mailing List <alsa-devel@alsa-project.org>,
	Takashi Iwai <tiwai@suse.com>,
	Baolin Wang <baolin.wang@linaro.org>,
	y2038 Mailman List <y2038@lists.linaro.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	Mark Brown <broonie@kernel.org>,
	Baolin Wang <baolin.wang7@gmail.com>
Subject: Re: [alsa-devel] [PATCH v6 5/8] ALSA: Avoid using timespec for struct snd_rawmidi_status
Date: Tue, 12 Nov 2019 21:26:00 +0100	[thread overview]
Message-ID: <s5hmud07sfb.wl-tiwai@suse.de> (raw)
In-Reply-To: <CAK8P3a0esv0T6ALMXJW40B45Qy5BqVSV9rhSD_sVNUc1T+suEg@mail.gmail.com>

On Tue, 12 Nov 2019 21:04:10 +0100,
Arnd Bergmann wrote:
> 
> On Tue, Nov 12, 2019 at 5:38 PM Takashi Iwai <tiwai@suse.de> wrote:
> > On Tue, 12 Nov 2019 16:16:39 +0100, Arnd Bergmann wrote:
> > > +#ifndef __KERNEL__
> > >  struct snd_rawmidi_status {
> > >       int stream;
> > > +     unsigned char pad1[sizeof(time_t) - sizeof(int)];
> > >       struct timespec tstamp;         /* Timestamp */
> > >       size_t avail;                   /* available bytes */
> > >       size_t xruns;                   /* count of overruns since last status (in bytes) */
> > >       unsigned char reserved[16];     /* reserved for future use */
> > >  };
> >
> > Can we use union instead of padding?  Something like:
> >
> > struct snd_rawmidi_status {
> >         union {
> >                 int stream;
> >                 time_t stream_alignment;
> >         };
> >         struct timespec tstamp;         /* Timestamp */
> >         ....
> 
> I think this would work most of the time, though I don't feel this is more
> readable than the other version.
> 
> More importantly, it requires compiling user applications with GNU extensions
> (--std=gnu89 or higher) or C11, but not C99, so this could be a problem
> for some applications.

OK, fair enough.

> If you feel there is a problem with the padding syntax, how about enclosing
> it in a typedef like:
> 
> typedef struct { char pad[sizeof(time_t) - sizeof(int)]; } __time_pad;
> 
> This typedef could be used in several structures from the other patches
> as well.

Yes, that improves the readability.


thanks,

Takashi

  reply	other threads:[~2019-11-12 20:26 UTC|newest]

Thread overview: 52+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-11-12 15:16 [alsa-devel] [PATCH v6 0/8] Fix year 2038 issue for sound subsystem Arnd Bergmann
2019-11-12 15:16 ` Arnd Bergmann
2019-11-12 15:16 ` [alsa-devel] [PATCH v6 1/8] ALSA: Replace timespec with timespec64 Arnd Bergmann
2019-11-12 15:16   ` Arnd Bergmann
2019-11-12 15:16 ` [alsa-devel] [PATCH v6 2/8] ALSA: Avoid using timespec for struct snd_timer_status Arnd Bergmann
2019-11-12 15:16   ` Arnd Bergmann
2019-11-12 15:42   ` [alsa-devel] " Takashi Iwai
2019-11-12 15:42     ` Takashi Iwai
2019-11-12 20:08     ` [alsa-devel] " Arnd Bergmann
2019-11-12 20:08       ` Arnd Bergmann
2019-11-12 20:28       ` [alsa-devel] " Takashi Iwai
2019-11-12 20:28         ` Takashi Iwai
2019-11-13 15:09         ` [alsa-devel] " Arnd Bergmann
2019-11-13 15:09           ` Arnd Bergmann
2019-11-12 15:16 ` [alsa-devel] [PATCH v6 3/8] ALSA: Avoid using timespec for struct snd_ctl_elem_value Arnd Bergmann
2019-11-12 15:16   ` Arnd Bergmann
2019-11-12 15:16 ` [alsa-devel] [PATCH v6 4/8] ALSA: Avoid using timespec for struct snd_pcm_status Arnd Bergmann
2019-11-12 15:16   ` Arnd Bergmann
2019-11-12 15:16 ` [alsa-devel] [PATCH v6 5/8] ALSA: Avoid using timespec for struct snd_rawmidi_status Arnd Bergmann
2019-11-12 15:16   ` Arnd Bergmann
2019-11-12 16:38   ` [alsa-devel] " Takashi Iwai
2019-11-12 16:38     ` Takashi Iwai
2019-11-12 20:04     ` Arnd Bergmann
2019-11-12 20:04       ` Arnd Bergmann
2019-11-12 20:26       ` Takashi Iwai [this message]
2019-11-12 20:26         ` Takashi Iwai
2019-11-13 15:16         ` Arnd Bergmann
2019-11-13 15:16           ` Arnd Bergmann
2019-11-12 15:16 ` [alsa-devel] [PATCH v6 6/8] ALSA: Avoid using timespec for struct snd_timer_tread Arnd Bergmann
2019-11-12 15:16   ` Arnd Bergmann
2019-11-12 15:16 ` [alsa-devel] [PATCH v6 7/8] ALSA: move snd_pcm_ioctl_sync_ptr_compat into pcm_native.c Arnd Bergmann
2019-11-12 15:16   ` Arnd Bergmann
2019-11-12 15:16 ` [alsa-devel] [PATCH v6 8/8] ALSA: add new 32-bit layout for snd_pcm_mmap_status/control Arnd Bergmann
2019-11-12 15:16   ` Arnd Bergmann
2019-11-15 15:24   ` [alsa-devel] " Arnd Bergmann
2019-11-15 15:24     ` Arnd Bergmann
2019-11-12 20:37 ` [alsa-devel] [PATCH v6 0/8] Fix year 2038 issue for sound subsystem Takashi Iwai
2019-11-12 20:37   ` Takashi Iwai
2019-11-13 14:32   ` [alsa-devel] " Arnd Bergmann
2019-11-13 14:32     ` Arnd Bergmann
2019-11-13 16:40     ` [alsa-devel] " Takashi Iwai
2019-11-13 16:40       ` Takashi Iwai
2019-11-13 16:51       ` [alsa-devel] " Arnd Bergmann
2019-11-13 16:51         ` Arnd Bergmann
2019-11-13 17:04         ` [alsa-devel] " Takashi Iwai
2019-11-13 17:04           ` Takashi Iwai
2019-11-13 17:19           ` [alsa-devel] " Arnd Bergmann
2019-11-13 17:19             ` Arnd Bergmann
2019-11-13 18:13             ` [alsa-devel] " Takashi Iwai
2019-11-13 18:13               ` Takashi Iwai
2019-11-13 20:48               ` [alsa-devel] " Arnd Bergmann
2019-11-13 20:48                 ` Arnd Bergmann

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=s5hmud07sfb.wl-tiwai@suse.de \
    --to=tiwai@suse.de \
    --cc=alsa-devel@alsa-project.org \
    --cc=arnd@arndb.de \
    --cc=baolin.wang7@gmail.com \
    --cc=baolin.wang@linaro.org \
    --cc=broonie@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=tiwai@suse.com \
    --cc=y2038@lists.linaro.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.