On Fri, 20 Feb 2026 12:25:43 -0300
Daniel Almeida <[email protected]> wrote:
> >> +
> >> + // Checks and updates the seat state based on the slot it points to
> >> + // (if any). Returns a Seat with a value matching the slot state.
> >> + fn check_seat(&mut self, locked_seat: &LockedSeat<T, MAX_SLOTS>) ->
> >> Seat {
> >> + let new_seat = match locked_seat.access(self) {
> >> + Seat::Active(seat_info) => {
> >> + let old_slot_idx = seat_info.slot as usize;
> >> + let slot = &self.slots[old_slot_idx];
> >> +
> >> + if kernel::warn_on!(
> >> + !matches!(slot, Slot::Active(slot_info) if
> >> slot_info.seqno == seat_info.seqno)
> >> + ) {
> >> + Seat::NoSeat
> >> + } else {
> >> + Seat::Active(SeatInfo {
> >> + slot: seat_info.slot,
> >> + seqno: seat_info.seqno,
> >> + })
> >> + }
> >> + }
> >> +
> >> + Seat::Idle(seat_info) => {
> >> + let old_slot_idx = seat_info.slot as usize;
> >> + let slot = &self.slots[old_slot_idx];
> >> +
> >> + if !matches!(slot, Slot::Idle(slot_info) if
> >> slot_info.seqno == seat_info.seqno) {
> >> + Seat::NoSeat
> >> + } else {
> >> + Seat::Idle(SeatInfo {
> >> + slot: seat_info.slot,
> >> + seqno: seat_info.seqno,
> >> + })
> >> + }
> >> + }
> >> +
> >> + _ => Seat::NoSeat,
> >> + };
> >> +
> >> + // FIXME: Annoying manual copy. The original idea was to not add
> >> Copy+Clone to SeatInfo,
> >> + // so that only slot.rs can change the seat state, but there
> >> might be better solutions
> >> + // to prevent that.
> >
> > Okay, I guess we want some inputs from Daniel and/or Alice on that one.
>
> Hm, I'd say we shouldn't implement Clone to avoid any possibility of holding
> on
> to stale state by duplicating it.
Okay, so basically what we have now.
>
> Why do we need to return Seat from this function? Can't we simply write
> locked_seat in place and not return anything?
We do both actually. IIRC, the reason is that LockedBy borrows &self if
we want to read the locked_seat, which prevents us from calling methods
taking a &mut ref from a `match(locked_seat.access())`.
>
> >
> >> + match &new_seat {
> >> + Seat::Active(seat_info) => {
> >> + *locked_seat.access_mut(self) = Seat::Active(SeatInfo {
> >> + slot: seat_info.slot,
> >> + seqno: seat_info.seqno,
> >> + })
> >> + }
> >> + Seat::Idle(seat_info) => {
> >> + *locked_seat.access_mut(self) = Seat::Idle(SeatInfo {
> >> + slot: seat_info.slot,
> >> + seqno: seat_info.seqno,
> >> + })
> >> + }
> >> + _ => *locked_seat.access_mut(self) = Seat::NoSeat,
> >> + }
> >> +
> >> + new_seat
> >> + }
> >> +
>
