Re: [Qemu-devel] [PATCH v3 1/4] adb-keys.h: initial commit

[Programmingkid]

On Mar 5, 2016, at 7:02 PM, Eric Blake wrote:

> On 03/04/2016 10:15 PM, Programmingkid wrote:
>> This commit implements the adb-keys.h file. It holds information on adb 
>> keycode
>> values.
>> 
>> Signed-off-by: John Arbuckle <address@hidden>
>> ---
>> Changed name of file from MacKeys.h to adb-keys.h.
>> Changed name of constants from MAC_KEYS_ to ADB_KEYS_. 
>> 
>> include/hw/input/adb-keys.h | 160 
>> ++++++++++++++++++++++++++++++++++++++++++++
>> 1 file changed, 160 insertions(+)
>> create mode 100644 include/hw/input/adb-keys.h
> 
> Thanks; you're getting closer at decent submission style.  Your
> threading is still broken, though (the mail is missing an 'In-Reply-To:'
> header that refers to <address@hidden>
> from the 0/4 cover letter); you can see this in the list archives:
> https://lists.gnu.org/archive/html/qemu-devel/2016-03/threads.html#01180
> 
> Note how your messages each appear as top-level threads, while most
> other series are properly threaded.

I guess there is still room for improvement. :)

> 
>> 
>> diff --git a/include/hw/input/adb-keys.h b/include/hw/input/adb-keys.h
>> new file mode 100644
>> index 0000000..6e009ee
>> --- /dev/null
>> +++ b/include/hw/input/adb-keys.h
>> @@ -0,0 +1,160 @@
>> +/*
>> + * QEMU System Emulator
>> + *
>> + * Copyright (c) 2003-2008 Fabrice Bellard
> 
> Fabrice did not write this file.  You're probably better off replacing
> this line wholesale with a Copyright 2016 and claiming copyright
> yourself, since it is new material from you that is not substantially
> copied from some existing file.

Ok. 

> 
>> + *
>> + * Permission is hereby granted, free of charge, to any person obtaining a 
>> copy
>> + * of this software and associated documentation files (the "Software"), to 
>> deal
>> + * in the Software without restriction, including without limitation the 
>> rights
>> + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
>> + * copies of the Software, and to permit persons to whom the Software is
>> + * furnished to do so, subject to the following conditions:
> 
> Any reason you chose the weaker BSD license instead of the
> project-default of GPLv2+?

I just used the text that was in vl.c. I thought all files in QEMU were using 
GPLv2. 

> 
>> +/*
>> + *  adb-keys.h
>> + *
>> + *  Provides a enum of all the Macintosh keycodes.
> 
> s/a enum/an enum/

Good catch.

> 
> Can you provide a URL to the source document that you used for coming up
> with this list, so that we can better double-check your work?  (Do it as
> a comment in this file, if you have one)

http://stackoverflow.com/questions/3202629/where-can-i-find-a-list-of-mac-virtual-key-codes

> 
>> + *  Note: keys like Power, volume related, and eject are handled at a lower
>> + *        level and are not available to QEMU. That doesn't mean we can't
>> + *        substitute one key for another. The function keys like F1 make a 
>> good
>> + *        substitute for these keys. This can be done in the GTK, SDL, or 
>> Cocoa
>> + *        code.
>> + */
>> +
>> +#ifndef __ADBKEYS__
>> +#define __ADBKEYS__
> 
> The name '__ADBKEYS__' is reserved for the compiler's use; you risk a
> future collision for no good reason.  It's fine to use 'ADB_KEYS_H' as
> your witness, better matching your filename and staying out of reserved
> namespace.

Ok.