Re: [PATCH v6 0/8] Fix year 2038 issue for sound subsystem

[Takashi Iwai]

On Wed, 13 Nov 2019 18:19:56 +0100,
Arnd Bergmann wrote:
> 
> On Wed, Nov 13, 2019 at 6:04 PM Takashi Iwai <tiwai@xxxxxxx> wrote:
> >
> > On Wed, 13 Nov 2019 17:51:57 +0100,
> > Arnd Bergmann wrote:
> > >
> > > On Wed, Nov 13, 2019 at 5:40 PM Takashi Iwai <tiwai@xxxxxxx> wrote:
> > > > On Wed, 13 Nov 2019 15:32:44 +0100, Arnd Bergmann wrote:
> > >
> > > > > We had discussed alternatives for this one last time, and decided
> > > > > to go with the solution I posted here. The main alternative would
> > > > > be to change the 'timespec' in snd_timer_tread to a fixed-length
> > > > > structure based on two 'long' members. This would avoid the
> > > > > need to match the command with the time_t type, but the cost would
> > > > > be requiring CLOCK_MONOTONIC timestamps to avoid the
> > > > > overflow, and changing all application source code that requires
> > > > > the type to be compatible with 'timespec'.
> > > >
> > > > Fair enough.
> > > >
> > > > One thing I forgot to mention: when we add/modify the ioctl or ABI, we
> > > > need to increment the protocol version, e.g. SNDRV_PCM_VERSION to
> > > > indicate user-space the supported ABI.  Please change these in your
> > > > next patches, too.
> > >
> > > Just to confirm: this should be a simple one-line patch at the end of the
> > > series like
> > >
> > > diff --git a/tools/include/uapi/sound/asound.h
> > > b/tools/include/uapi/sound/asound.h
> > > index df1153cea0b7..72e8380c6dcd 100644
> > > --- a/include/uapi/sound/asound.h
> > > +++ b/include/uapi/sound/asound.h
> > > @@ -154,7 +154,7 @@ struct snd_hwdep_dsp_image {
> > >   *                                                                           *
> > >   *****************************************************************************/
> > >
> > > -#define SNDRV_PCM_VERSION              SNDRV_PROTOCOL_VERSION(2, 0, 14)
> > > +#define SNDRV_PCM_VERSION              SNDRV_PROTOCOL_VERSION(2, 0, 15)
> > >
> > >  typedef unsigned long snd_pcm_uframes_t;
> > >  typedef signed long snd_pcm_sframes_t;
> > >
> > > right? Most other kernel interfaces have no version numbering, so
> > > I don't know what policy you have here.
> >
> > I don't mind much about that, so it's up to you -- we can fold this
> > change into the patch that actually adds or modifies the ioctl, too.
> 
> I've added the patch below to the end of the series now, will repost
> the series  after no more comments come in for this version.
> Having a single patch to update the version seems better than updating
> it multiple times with each patch touching the ABI in this series.
> 
>       Arnd
> 
> commit b83a3eaece9b445f897a6f5ac2a903f2566a0e9d
> Author: Arnd Bergmann <arnd@xxxxxxxx>
> Date:   Wed Nov 13 17:49:14 2019 +0100
> 
>     ALSA: bump uapi version number
> 
>     Change SNDRV_PCM_VERSION to indicate the addition of the time64
>     version of the mmap interface and these ioctl commands:
> 
>     SNDRV_PCM_IOCTL_SYNC
>     SNDRV_RAWMIDI_IOCTL_STATUS
>     SNDRV_PCM_IOCTL_STATUS
>     SNDRV_PCM_IOCTL_STATUS_EXT
>     SNDRV_TIMER_IOCTL_TREAD
>     SNDRV_TIMER_IOCTL_STATUS
> 
>     32-bit applications built with 64-bit time_t require both the headers
>     and the running kernel to support at least API version 2.0.15. When
>     built with earlier kernel headers, some of these may not work
>     correctly, so applications are encouraged to fail compilation like
> 
>      #if SNDRV_PCM_VERSION < SNDRV_PROTOCOL_VERSION(2, 0, 15)
>      extern int __fail_build_for_time_64[sizeof(long) - sizeof(time_t)];
>      #endif
> 
>     or provide their own updated copy of the header file.
> 
>     Signed-off-by: Arnd Bergmann <arnd@xxxxxxxx>
> 
> diff --git a/include/uapi/sound/asound.h b/include/uapi/sound/asound.h
> index 25dbf71fa667..5cddfe62c97a 100644
> --- a/include/uapi/sound/asound.h
> +++ b/include/uapi/sound/asound.h
> @@ -156,7 +156,7 @@ struct snd_hwdep_dsp_image {
>   *                                                                           *
>   *****************************************************************************/
> 
> -#define SNDRV_PCM_VERSION              SNDRV_PROTOCOL_VERSION(2, 0, 14)
> +#define SNDRV_PCM_VERSION              SNDRV_PROTOCOL_VERSION(2, 0, 15)
> 
>  typedef unsigned long snd_pcm_uframes_t;
>  typedef signed long snd_pcm_sframes_t;

I wasn't clear enough.  Each ALSA device API has a protocol version,
not only PCM, so we need updating SNDRV_RAWMIDI_VERSION and
SNDRV_TIMER_VERSION as well.


thanks,

Takashi