On Mon, 12 Apr 2021 12:26:49 +0200,
Jaroslav Kysela wrote:
>
> Dne 12. 04. 21 v 12:04 Takashi Iwai napsal(a):
> > On Mon, 12 Apr 2021 11:43:02 +0200,
> > Jaroslav Kysela wrote:
> >>
> >> Dne 12. 04. 21 v 11:31 Takashi Iwai napsal(a):
> >>> On Sun, 11 Apr 2021 19:52:21 +0200,
> >>> Jaroslav Kysela wrote:
> >>>>>>> struct snd_rawmidi_params {
> >>>>>>> int stream;
> >>>>>>> size_t buffer_size; /* queue size in bytes */
> >>>>>>> size_t avail_min; /* minimum avail bytes for wakeup */
> >>>>>>> unsigned int no_active_sensing: 1; /* do not send active sensing byte in close() */
> >>>>>>> - unsigned char reserved[16]; /* reserved for future use */
> >>>>>>> + unsigned char framing; /* For input data only, frame incoming data */
> >>>>>> We can move this flag to above 32-bit word (no_active_sensing). I'm not sure,
> >>>>>> if we need 8 bits for this. It's first change after 20 years. Another flag may
> >>>>>> obsolete this one.
> >>>>>
> >>>>> Not sure what you mean by this, could you show the code? Framing is an
> >>>>> enum rather than a flag, in case we find other framing formats with
> >>>>> other sizes that would obsolete this one.
> >>>>
> >>>> unsigned int no_active_sensing: 1;
> >>>> unsigned int framing32: 1;
> >>>> unsigned int framing64: 1; /* future extension, framing32 == 0 in this case */
> >>>>
> >>>> or:
> >>>>
> >>>> unsigned int framing_mode: 3; /* three bits for future types */
> >>>>
> >>>> perhaps also:
> >>>>
> >>>> unsigned int clock_type: 3; /* three bits for the clock type selection */
> >>>
> >>> The usage of bit fields in an ioctl struct is a bad idea from the
> >>> compat layer POV. Let's avoid it.
> >>
> >> What exactly do you mean?
> >
> > The compat layer has the hard time to deal with the conversion of bit
> > fields to a different struct. And, it's a nightmare for the emulator
> > like QEMU. If it has to convert the ioctl struct, it has to consider
> > about the byte order as well (and there is no strict definition how to
> > put the bit fields in C language).
>
> The endianness of the 32-bit word can be changed quite easily - and the
> situation is similar to bit flags stored in the 32-bit word. Nobody prevents
> QEMU to work with the whole 32-bit word instead bit fields in this case.
And how you assure that the bit field is set to which bit position?
There is no guarantee defined in C language about that.
> The linux compat layer may copy the whole 32-bit word, too.
>
> I think that your argument is not so strong here.
Oh please, this is my experience years ago while discussing with the
QEMU developers in the past. Using a bit field for an ioctl struct is
a horrible idea, after all.
> > That said, a bit field can be useful for the internal object
> > definition (if there is no racy access), but not for an object for the
> > communication that may be interpreted among different architectures.
>
> In this case, we have 31 free bits and this information can be stored there. I
> would prefer to keep the reserved bytes for some large fields.
Again, C language doesn't define the position of the bit fields.
That's the primary problem.
If we really have to save the space, it's a nice workaround. But for
other purposes, there is really little merit and more flip side by
that.
Takashi
