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