[netcf-devel] [PATCH 5/5] Conditionalize Windows portion
Eric Blake
eblake at redhat.com
Wed Sep 22 20:17:48 UTC 2010
On 09/17/2010 10:29 AM, Adam Stokes wrote:
> ---
> src/drv_initscripts.c | 2 +-
> src/internal.h | 3 ++
> src/ncftool.c | 7 ++++++
> src/netcf.c | 54 ++++++++++++++++++++++++++++++++++++++++++++++++-
> src/xslt_ext.c | 2 +-
> 5 files changed, 65 insertions(+), 3 deletions(-)
>
> diff --git a/src/drv_initscripts.c b/src/drv_initscripts.c
> index c0bcfd5..2127d2a 100644
> --- a/src/drv_initscripts.c
> +++ b/src/drv_initscripts.c
> @@ -100,7 +100,7 @@ static const struct augeas_xfm_table augeas_xfm_iptables =
>
> static const char *const prog_lokkit = "/usr/sbin/lokkit";
> static const char *const lokkit_custom_rules =
> - "--custom-rules=ipv4:filter:" DATADIR "/netcf/iptables-forward-bridged";
> + "--custom-rules=ipv4:filter:" NETCF_DATADIR "/netcf/iptables-forward-bridged";
I still don't see why you had to do this rename. A commit comment
explaining why would be great, and it may be possible to do things
without a rename.
>
> static const char *const prog_rc_d_iptables = "/etc/init.d/iptables";
>
> diff --git a/src/internal.h b/src/internal.h
> index d06665d..1b1726c 100644
> --- a/src/internal.h
> +++ b/src/internal.h
> @@ -83,6 +83,9 @@
> #define ATTRIBUTE_NOINLINE
> #endif /* __GNUC__ */
>
> +#ifndef WIN32
> +#define DATADIR NETCF_DATADIR
> +#endif
> /* This needs ATTRIBUTE_RETURN_CHECK */
> #include "ref.h"
>
> diff --git a/src/ncftool.c b/src/ncftool.c
> index 2cb139d..6cdbdef 100644
> --- a/src/ncftool.c
> +++ b/src/ncftool.c
> @@ -677,7 +677,11 @@ static void parse_opts(int argc, char **argv) {
> while ((opt = getopt_long(argc, argv, "+dhr:", options,&idx)) != -1) {
> switch(opt) {
> case 'd':
> +#ifdef WIN32
> + _putenv("NETCF_DEBUG=1");
> +#else
> setenv("NETCF_DEBUG", "1", 1);
> +#endif
NACK to this hunk; I'd rather get gnulib to relax the 'setenv' module to
LGPLv2+ and rely on that (I've just sent an email to the gnulib list
requesting that).
> break;
> case 'h':
> usage();
> @@ -784,7 +788,10 @@ int main(int argc, char **argv) {
> fprintf(stderr, "Failed to initialize netcf\n");
> if (ncf != NULL)
> print_netcf_error();
> +#ifndef WIN32
> + // FIXME: Need to find out why ncf_init is croaking
> exit(EXIT_FAILURE);
> +#endif
It would be nicer to find out why before this commit goes in. But even
if we don't figure out why, this reads awkwardly to me. How about:
#ifdef WIN32
// FIXME: ncf_init is always failing on WIN32, but we can proceed
// anyways. Find out why and simplify this code.
#else
exit(EXIT_FAILURE);
#endif
> }
> readline_init();
> if (optind< argc) {
> diff --git a/src/netcf.c b/src/netcf.c
> index 1a95332..9ec6dea 100644
> --- a/src/netcf.c
> +++ b/src/netcf.c
> @@ -35,7 +35,11 @@
>
> #include "internal.h"
> #include "netcf.h"
> +#ifdef WIN32
> +#include "netcf_win.h"
> +#else
> #include "dutil.h"
> +#endif
>
> /* Clear error code and details */
> #define API_ENTRY(ncf) \
> @@ -88,7 +92,11 @@ int ncf_init(struct netcf **ncf, const char *root) {
> if (make_ref(*ncf)< 0)
> goto oom;
> if (root == NULL)
> +#ifdef WIN32
> + root = "c:\\";
> +#else
> root = "/";
> +#endif
Rather than having an in-function #ifdef, it would be nicer to have a
#define earlier in the file:
#ifdef WIN32
# define ROOT "c:\\"
#else
# define ROOT "/"
#endif
Also, is the C: drive always the right root?
> if (root[strlen(root)-1] == '/') {
> (*ncf)->root = strdup(root);
This code is skipped given your WIN32 definition of root; but your root
is still absolute. Is that a problem?
> } else {
> @@ -99,9 +107,14 @@ int ncf_init(struct netcf **ncf, const char *root) {
> goto oom;
> (*ncf)->data_dir = getenv("NETCF_DATADIR");
> if ((*ncf)->data_dir == NULL)
> - (*ncf)->data_dir = DATADIR "/netcf";
> + (*ncf)->data_dir = NETCF_DATADIR "/netcf";
> (*ncf)->debug = getenv("NETCF_DEBUG") != NULL;
> +#ifdef WIN32
> + return 0;
> +#else
> + /* Needs investigation on WIN32 */
Move this comment into the WIN32 block.
> return drv_init(*ncf);
> +#endif
> oom:
> ncf_close(*ncf);
> *ncf = NULL;
> @@ -151,12 +164,16 @@ struct netcf_if * ncf_lookup_by_name(struct netcf *ncf, const char *name) {
> return drv_lookup_by_name(ncf, name);
> }
>
> +#ifdef WIN32
> +int ncf_lookup_by_mac_string() { return -1; }
> +#else
> int
> ncf_lookup_by_mac_string(struct netcf *ncf, const char *mac,
> int maxifaces, struct netcf_if **ifaces) {
> API_ENTRY(ncf);
> return drv_lookup_by_mac_string(ncf, mac, maxifaces, ifaces);
> }
> +#endif
>
> /*
> * Define/start/stop/undefine interfaces
> @@ -180,10 +197,15 @@ const char *ncf_if_mac_string(struct netcf_if *nif) {
> }
>
> /* Delete the definition */
> +#ifdef WIN32
> +/* No mingw implementation */
> +int ncf_if_undefine() { return -1; };
> +#else
> int ncf_if_undefine(struct netcf_if *nif) {
> API_ENTRY(nif->ncf);
> return drv_undefine(nif);
> }
> +#endif
>
> /* Bring the interface up */
> int ncf_if_up(struct netcf_if *nif) {
> @@ -202,28 +224,43 @@ int ncf_if_down(struct netcf_if *nif) {
> /* Produce an XML description for the interface, in the same format that
> * NCF_DEFINE expects
> */
> +#ifdef WIN32
> +/* No mingw implementation */
> +char *ncf_if_xml_desc() { return NULL; }
> +#else
> char *ncf_if_xml_desc(struct netcf_if *nif) {
> API_ENTRY(nif->ncf);
> return drv_xml_desc(nif);
> }
> +#endif
>
> /* Produce an XML description of the current live state of the
> * interface, in the same format that NCF_DEFINE expects, but
> * potentially with extra info not contained in the static config (ie
> * the current IP address of an interface that uses DHCP)
> */
> +#ifdef WIN32
> +/* No mingw implementation */
> +char *ncf_if_xml_state() { return NULL; }
> +#else
> char *ncf_if_xml_state(struct netcf_if *nif) {
> API_ENTRY(nif->ncf);
> return drv_xml_state(nif);
> }
> +#endif
>
> /* Report various status info about the interface as bits in
> * "flags". Returns 0 on success, -1 on failure
> */
> +#ifdef WIN32
> +/* No mingw implementation */
> +int ncf_if_status() { return -1; }
> +#else
> int ncf_if_status(struct netcf_if *nif, unsigned int *flags) {
> API_ENTRY(nif->ncf);
> return drv_if_status(nif, flags);
> }
> +#endif
>
> /* Release any resources used by this NETCF_IF; the pointer is invalid
> * after this call
> @@ -250,18 +287,32 @@ int ncf_error(struct netcf *ncf, const char **errmsg, const char **details) {
> /*
> * Test interface
> */
> +#ifdef WIN32
> +/* No mingw implementation */
> +int ncf_get_aug() { return -1; }
> +#else
> int ncf_get_aug(struct netcf *ncf, const char *ncf_xml, char **aug_xml) {
> API_ENTRY(ncf);
>
> return drv_get_aug(ncf, ncf_xml, aug_xml);
> }
> +#endif
>
> +#ifdef WIN32
> +/* No mingw implementation */
> +int ncf_put_aug() { return -1; }
> +#else
> int ncf_put_aug(struct netcf *ncf, const char *aug_xml, char **ncf_xml) {
> API_ENTRY(ncf);
>
> return drv_put_aug(ncf, aug_xml, ncf_xml);
> }
> +#endif
>
> +#ifndef WIN32
> +/* Do not see any other way around this as there is no
> + * portable option for strerror_r or pthread_sigmask
> + */
> /*
> * Internal helpers
> */
> @@ -395,6 +446,7 @@ error:
> return ret;
> }
>
> +#endif /* WIN32 */
> /*
> * argv_to_string() is borrowed from libvirt's
> * src/util.c:virArgvToString()
> diff --git a/src/xslt_ext.c b/src/xslt_ext.c
> index 07e2b41..444889a 100644
> --- a/src/xslt_ext.c
> +++ b/src/xslt_ext.c
> @@ -106,7 +106,7 @@ static void ipcalc_prefix(xmlXPathParserContextPtr ctxt, int nargs) {
> goto error;
> }
>
> - r = inet_aton((char *) netmask_str,&netmask);
> + r = inet_pton(AF_INET, (char *) netmask_str,&netmask);
This hunk doesn't seem like it fits with the rest of the patch. Is it
an independent bug fix?
> if (r< 0) {
> xsltTransformError(xsltXPathGetTransformContext(ctxt), NULL, NULL,
> "ipcalc:prefix: illegal netmask '%s'",
--
Eric Blake eblake at redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org
More information about the netcf-devel
mailing list