Re: [libvirt] [PATCH 2/4 V4] util: add virtkey
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(a)redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org