[libvirt] PATCH: Generic internal API for network XML parser/formatter

[Jim Meyering]

"Daniel P. Berrange" <berrange at redhat.com> wrote:
...
> diff -r a6e5acdd23df src/network_conf.h
...
> +/* 2 possible types of forwarding */
> +enum virNetworkForwardType {
> +    VIR_NETWORK_FORWARD_NONE   = 0,
> +    VIR_NETWORK_FORWARD_NAT,
> +    VIR_NETWORK_FORWARD_ROUTE,
> +
> +    VIR_NETWORK_FORWARD_LAST,
> +};
> +
> +typedef struct _virNetworkDHCPRangeDef virNetworkDHCPRangeDef;
> +typedef virNetworkDHCPRangeDef *virNetworkDHCPRangeDefPtr;
> +struct _virNetworkDHCPRangeDef {
> +    char *start;
> +    char *end;
> +};
> +
> +typedef struct _virNetworkDef virNetworkDef;
> +typedef virNetworkDef *virNetworkDefPtr;
> +struct _virNetworkDef {
> +    unsigned char uuid[VIR_UUID_BUFLEN];
> +    char *name;
> +
> +    char *bridge;       /* Name of bridge device */
> +    int stp : 1; /* Spanning tree protocol */
> +    long delay;   /* Bridge forward delay (ms) */

It looks like delay can only be non-negative, so "unsigned long" would work.
The only change that'd have to accompany it would be s/%ld/%lu/
in a printf format string.

> +    int forwardType;    /* One of virNetworkForwardType constants */

How about using enum virNetworkForwardType?
Nicer for debugging.

> +    char *forwardDev;   /* Destination device for forwarding */
> +
> +    char *ipAddress;    /* Bridge IP address */
> +    char *netmask;
> +    char *network;
> +
> +    int nranges;        /* Zero or more dhcp ranges */

This is never negative, so could be "unsigned int".

...
> +struct _virNetworkObj {
> +    int dnsmasqPid;

How about pid_t?

> +    unsigned int active : 1;
> +    unsigned int autostart : 1;
> +    unsigned int persistent : 1;
> +
> +    char *configFile;    /* Persistent config file path */
> +    char *autostartLink; /* Symlink path for autostart */
> +
> +    virNetworkDefPtr def; /* The current definition */
> +    virNetworkDefPtr newDef; /* New definition to activate at shutdown */
> +
> +    virNetworkObjPtr next;
> +};