On Mar 1, 2016, at 7:25 PM, Eric Blake wrote: > On 03/01/2016 05:12 PM, Programmingkid wrote: > >>> >>>> + >>>> + [MAC_KEY_ESC] = Q_KEY_CODE_ESC, >>>> + //[MAC_KEY_F1] = Q_KEY_CODE_POWER, // Just in case you need the power >>>> key >>>> + [MAC_KEY_F1] = Q_KEY_CODE_F1, >>> >>> The comment looks weird. Probably worth a mention in the commit message >>> why you need it. >> >> Maybe this: >> >> [MAC_KEY_ESC] = Q_KEY_CODE_ESC, >> //[MAC_KEY_F1] = Q_KEY_CODE_POWER, // You might need this key if you use >> Macsbug. >> [MAC_KEY_F1] = Q_KEY_CODE_F1, > > Not a code comment, but a commit message comment stating why you aren't > providing a mapping for q_KEY_CODE_POWER. For that matter, I don't > think a code comment is useful, just nuke the entire line (the comment > doesn't add anything). > >> >> >>> >>>> >>>> static int cocoa_keycode_to_qemu(int keycode) >>>> { >>>> - if (ARRAY_SIZE(keymap) <= keycode) { >>>> + if (ARRAY_SIZE(macToQKeyCodeMap) <= keycode) { >>>> fprintf(stderr, "(cocoa) warning unknown keycode 0x%x\n", keycode); >>> >>> Pre-existing, but we should fix this to avoid fprintf. >> >> What do you mean by pre-existing? > > You weren't the original cause of the bug, so it is not necessarily this > patch's job to fix the bug. Therefore, "pre-existing". But since the > bug was observed during review of your patch, you may want to fix it > anyways, probably as a separate patch.
So you want this: if (ARRAY_SIZE(macToQKeyCodeMap) <= keycode) { error_report("(cocoa) warning unknown keycode 0x%x\n", keycode);