hid: clarify hid_keyboard_process_keycode
Coverity thinks the fallthroughs are smelly. They are correct, but everything else in this function is like "wut?". Refer explicitly to bits 8 and 9 of hs->kbd.modifiers instead of shifting right first and using (1 << 7). Document what the scancode is when hid_code is 0xe0. And add plenty of comments. Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
This commit is contained in:
		
							parent
							
								
									e2f6bac301
								
							
						
					
					
						commit
						562f93754b
					
				| @ -239,7 +239,7 @@ static void hid_keyboard_event(DeviceState *dev, QemuConsole *src, | ||||
| 
 | ||||
| static void hid_keyboard_process_keycode(HIDState *hs) | ||||
| { | ||||
|     uint8_t hid_code, key; | ||||
|     uint8_t hid_code, index, key; | ||||
|     int i, keycode, slot; | ||||
| 
 | ||||
|     if (hs->n == 0) { | ||||
| @ -249,7 +249,8 @@ static void hid_keyboard_process_keycode(HIDState *hs) | ||||
|     keycode = hs->kbd.keycodes[slot]; | ||||
| 
 | ||||
|     key = keycode & 0x7f; | ||||
|     hid_code = hid_usage_keys[key | ((hs->kbd.modifiers >> 1) & (1 << 7))]; | ||||
|     index = key | ((hs->kbd.modifiers & (1 << 8)) >> 1); | ||||
|     hid_code = hid_usage_keys[index]; | ||||
|     hs->kbd.modifiers &= ~(1 << 8); | ||||
| 
 | ||||
|     switch (hid_code) { | ||||
| @ -257,18 +258,41 @@ static void hid_keyboard_process_keycode(HIDState *hs) | ||||
|         return; | ||||
| 
 | ||||
|     case 0xe0: | ||||
|         assert(key == 0x1d); | ||||
|         if (hs->kbd.modifiers & (1 << 9)) { | ||||
|             hs->kbd.modifiers ^= 3 << 8; | ||||
|             /* The hid_codes for the 0xe1/0x1d scancode sequence are 0xe9/0xe0.
 | ||||
|              * Here we're processing the second hid_code.  By dropping bit 9 | ||||
|              * and setting bit 8, the scancode after 0x1d will access the | ||||
|              * second half of the table. | ||||
|              */ | ||||
|             hs->kbd.modifiers ^= (1 << 8) | (1 << 9); | ||||
|             return; | ||||
|         } | ||||
|         /* fall through to process Ctrl_L */ | ||||
|     case 0xe1 ... 0xe7: | ||||
|         /* Ctrl_L/Ctrl_R, Shift_L/Shift_R, Alt_L/Alt_R, Win_L/Win_R.
 | ||||
|          * Handle releases here, or fall through to process presses. | ||||
|          */ | ||||
|         if (keycode & (1 << 7)) { | ||||
|             hs->kbd.modifiers &= ~(1 << (hid_code & 0x0f)); | ||||
|             return; | ||||
|         } | ||||
|     case 0xe8 ... 0xef: | ||||
|         /* fall through */ | ||||
|     case 0xe8 ... 0xe9: | ||||
|         /* USB modifiers are just 1 byte long.  Bits 8 and 9 of
 | ||||
|          * hs->kbd.modifiers implement a state machine that detects the | ||||
|          * 0xe0 and 0xe1/0x1d sequences.  These bits do not follow the | ||||
|          * usual rules where bit 7 marks released keys; they are cleared | ||||
|          * elsewhere in the function as the state machine dictates. | ||||
|          */ | ||||
|         hs->kbd.modifiers |= 1 << (hid_code & 0x0f); | ||||
|         return; | ||||
| 
 | ||||
|     case 0xea ... 0xef: | ||||
|         abort(); | ||||
| 
 | ||||
|     default: | ||||
|         break; | ||||
|     } | ||||
| 
 | ||||
|     if (keycode & (1 << 7)) { | ||||
|  | ||||
		Loading…
	
	
			
			x
			
			
		
	
		Reference in New Issue
	
	Block a user
	 Paolo Bonzini
						Paolo Bonzini