Re: [PATCH] ALSA: document that struct __snd_pcm_mmap_control64 is messed up

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

 



On Thu, 06 Apr 2023 08:46:51 +0200,
Oswald Buddenhagen wrote:
> 
> On Thu, Apr 06, 2023 at 08:03:39AM +0200, Takashi Iwai wrote:
> > On Wed, 05 Apr 2023 22:12:19 +0200,
> > Oswald Buddenhagen wrote:
> >> 
> >> I suppose this can't be changed anymore due to binary compat concerns.
> > 
> > Yes, please check the thread at
> >  https://lore.kernel.org/all/29QBMJU8DE71E.2YZSH8IHT5HMH@xxxxxxxxxxx/
> > 
> >> +	__pad_before_uframe __pad2;	 // BUG: this should have been __pad_after_uframe!
> > 
> > Writing this alone doesn't help much.  Actual help would be to mention
> > that this typo is kept intentionally.
> > 
> hmm, my thinking is that the immediate response to reading that
> comment would be "so why don't you change it?!", at which point the
> person would either realize by themselves that this is subject to
> binary compat constraints, or would git-blame it and see the
> explanation.
> 
> anyway, my concern is keeping this *short*, so it doesn't distract.
> 
> maybe
> 
>   // BUG: should be __pad_after_uframe, but binary compat
> 
> would do, though obviously the grammar kinda sucks.
> 
> the (too) long version could be
> 
>   // BUG: this should be __pad_after_uframe, but
>   // binary compatibility constraints prevent a fix.
> 
> choose your death, i'll deliver it. ;-)

The "BUG:" suffix should be dropped.  This would catch eyes of (badly)
trained kernel programmers as if it were a kernel panic message :)

Also the term "binary compatibility" is ambiguous in this context --
especially because we're dealing with the code that treats the
32/64bit binary compatibility.

But, yeah, I'm for that direction.


thanks,

Takashi



[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