[libvirt] [PATCH 2/4 V4] util: add virtkey

[Eric Blake]

On 07/19/2011 09:04 AM, Daniel P. Berrange wrote:
> On Thu, Jul 14, 2011 at 11:32:16AM +0800, Lai Jiangshan wrote:
>> Add virtkey lib for usage-improvment and keycode translating.
>> Add 4 internal API for the aim
>>
>> +
>> +struct keycode {

>> +extern struct keycode virtKeycodes[];
>> +extern int virtKeycodesSize;
>
> None of these three declarations need to be in the header file
> since they are private impl details of the source fiel.

Well, as written in this patch, they are used in both virtkey.c, and by 
the generated virkeymaps.c.  But, a better idea would be:

virkey.h: public functions (virKeycodeSetToString, 
virKeycodeSetFromString [both from VIR_ENUM_DECL], 
virKeycodeValueFromString, virKeycodeValueTranslate)

virkey.c:
#include "virkey.h"
#define VIR_KEY_INTERNAL
struct keycode {} ...
#include "virkeymap.h"
implement the public functions

virkeymap-gen.pl: generates virkeymap.h

virkeymap.h:
#ifndef VIR_KEY_INTERNAL
# error do not use this; it is not a public header
#endif
static struct keycode keycodes[] = {
...
}

that way, the generated file is not a separate c file, but a chunk of 
included code to complete the single virkey.c.  See for example how 
remote_driver.c does a #include "remote_client_bodies.h" in the middle 
of the file, and make sure that the Makefile.am snippets for virkeymap.h 
(making sure it is in EXTRA_DIST and so forth) resemble the rules for 
remote_client_bodies.h.

I was debating about whipping this series into shape according to Dan's 
review comments, since you got acks on the other 3 out of 4, but the 
changes to patch 2 are looking to be significant enough to need a v5.

-- 
Eric Blake   eblake at redhat.com    +1-801-349-2682
Libvirt virtualization library http://libvirt.org