[netcf-devel] [PATCH 2/2] Make use of win32 routines where necessary

Laine Stump laine at laine.org
Tue Nov 9 17:34:54 UTC 2010


On 11/08/2010 02:02 PM, Adam Stokes wrote:
> - Here are changes in order to process the correct routines when run on Windows.
> - Detecting of Windows system drive can be accomplished through "SYSTEMDRIVE" environment variable
>    and falling back to C: drive as a last resort.
> ---
>   src/netcf.c |  100 +++++++++++++++++++++++++++++++++++++++++++++++++++++++---
>   1 files changed, 94 insertions(+), 6 deletions(-)
>
> diff --git a/src/netcf.c b/src/netcf.c
> index 1a95332..ac70f3c 100644
> --- a/src/netcf.c
> +++ b/src/netcf.c
> @@ -30,14 +30,22 @@
>   #include<unistd.h>
>   #include<sys/wait.h>
>   #include<signal.h>
> +#include<spawn.h>
>   #include<errno.h>
> -#include "safe-alloc.h"
>
> +#include "safe-alloc.h"
>   #include "internal.h"
>   #include "netcf.h"
> -#include "dutil.h"
> +#ifdef WIN32
> +# include "netcf_win.h"
> +#else
> +# include "dutil.h"
> +#endif
>
>   /* Clear error code and details */
> +#ifdef WIN32
> +#define API_ENTRY(ncf) NULL;
> +#else
>   #define API_ENTRY(ncf)                          \
>       do {                                        \
>           (ncf)->errcode = NETCF_NOERROR;         \
> @@ -45,6 +53,7 @@
>           if (ncf->driver != NULL)                \
>               drv_entry(ncf);                     \
>       } while(0);
> +#endif


The intent of API_ENTRY is to clear out the error code in the netcf 
object, as it will have been left set to the last error encountered on 
the previous call to a netcf function. This is not cleared out in any 
other manner (except by setting a new error), so it will not work to 
turn API_ENTRY() into a NOP.


>
>   /* Human-readable error messages. This array is indexed by NETCF_ERRCODE_T */
>   static const char *const errmsgs[] = {
> @@ -87,8 +96,15 @@ int ncf_init(struct netcf **ncf, const char *root) {
>       *ncf = NULL;
>       if (make_ref(*ncf)<  0)
>           goto oom;
> -    if (root == NULL)
> +    if (root == NULL) {
> +#ifdef WIN32
> +        root = getenv("SYSTEMDRIVE");
> +        if (!root)
> +            root = "c:";
> +#else
>           root = "/";
> +#endif
> +    }


It would be cleaner to turn this into a utility function in 
dutil_linux.c for linux, and in a Windows-specific file for the windows 
port. The less #ifdfefs there are in the middle of functions, the better.


>       if (root[strlen(root)-1] == '/') {
>           (*ncf)->root = strdup(root);
>       } else {
> @@ -99,9 +115,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 */
>       return drv_init(*ncf);
> +#endif

You should provide a drv_init() function in a Windows-specific file. 
(Actually, it looks like that's a problem all through netcf.c - rather 
than doing #ifdefs here, you need to provide your own replacement for 
drv_initscripts.c and dutil_linux.c.)

>    oom:
>       ncf_close(*ncf);
>       *ncf = NULL;
> @@ -116,8 +137,12 @@ int ncf_close(struct netcf *ncf) {
>
>       ERR_COND_BAIL(ncf->ref>  1, ncf, EINUSE);
>
> +#ifdef WIN32
> +    free(ncf->driver);
> +#else
>       drv_close(ncf);
>       unref(ncf, netcf);
> +#endif

Again, there should be a Windows-specific drv_close() in some other 
file. Also, you need to make unref work properly, rather than calling 
free directly.

>       return 0;
>    error:
>       return -1;
> @@ -151,23 +176,38 @@ 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(struct netcf *ncf, const char *mac,
> +                             int maxifaces, struct netcf_if **ifaces) {
> +    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


Please make a stub "drv_lookup_by_mac_string()" in your own 
WIndows-specific file (drv_mswindows.c?) rather than cluttering this 
file with #ifdefs.

I won't bother commenting on the rest of these #ifdefs.



>   /*
>    * Define/start/stop/undefine interfaces
>    */
>
>   /* Define a new interface */
> +#ifdef WIN32
> +struct netcf_if *
> +ncf_define(struct netcf *ncf, const char *xml) {
> +    API_ENTRY(ncf);
> +    return ncf;
> +}
> +#else
>   struct netcf_if *
>   ncf_define(struct netcf *ncf, const char *xml) {
>       API_ENTRY(ncf);
>       return drv_define(ncf, xml);
>   }
> +#endif
>
>   const char *ncf_if_name(struct netcf_if *nif) {
>       API_ENTRY(nif->ncf);
> @@ -180,10 +220,17 @@ const char *ncf_if_mac_string(struct netcf_if *nif) {
>   }
>
>   /* Delete the definition */
> +#ifdef WIN32
> +/* No mingw implementation */
> +int ncf_if_undefine(struct netcf_if *nif) {
> +    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 +249,49 @@ 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(struct netcf_if *nif) {
> +    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(struct netcf_if *nif) {
> +    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(struct netcf_if *nif, unsigned int *flags) {
> +    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
> @@ -240,6 +308,7 @@ int ncf_error(struct netcf *ncf, const char **errmsg, const char **details) {
>
>       if (ncf->errcode>= ARRAY_CARDINALITY(errmsgs))
>           errcode = NETCF_EINTERNAL;
> +


Gratuitous change in whitespace.


>       if (errmsg)
>           *errmsg = errmsgs[errcode];
>       if (details)
> @@ -250,22 +319,36 @@ int ncf_error(struct netcf *ncf, const char **errmsg, const char **details) {
>   /*
>    * Test interface
>    */
> +#ifdef WIN32
> +/* No mingw implementation */
> +int ncf_get_aug(struct netcf *ncf, const char *ncf_xml, char **aug_xml) {
> +    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(struct netcf *ncf, const char *aug_xml, char **ncf_xml) {
> +    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
>   /*
>    * Internal helpers
>    */
> -
>   static int


Another gratuitous whitespace change.


>   exec_program(struct netcf *ncf,
>                const char *const*argv,
> @@ -310,7 +393,6 @@ exec_program(struct netcf *ncf,
>
>       /* Clear out all signal handlers from parent so nothing unexpected
>          can happen in our child once we unblock signals */
> -
>       sig_action.sa_handler = SIG_DFL;


And again.


>       sig_action.sa_flags = 0;
>       sigemptyset(&sig_action.sa_mask);
> @@ -349,6 +431,7 @@ error:
>          should never jump here on error */
>       return -1;
>   }
> +#endif
>
>   /**
>    * Run a command without using the shell.
> @@ -368,8 +451,13 @@ int run_program(struct netcf *ncf, const char *const *argv) {
>       argv_str = argv_to_string(argv);
>       ERR_NOMEM(argv_str == NULL, ncf);
>
> +#ifdef WIN32
> +    ret = posix_spawnp(&childpid, argv[0], NULL, NULL, (char * const*)argv, environ);
> +    ERR_COND_BAIL(ret != 0, ncf, EOTHER);
> +#else
>       exec_program(ncf, argv, argv_str,&childpid);
>       ERR_BAIL(ncf);
> +#endif
>
>       while ((waitret = waitpid(childpid,&exitstatus, 0) == -1)&&
>              errno == EINTR) {



More information about the netcf-devel mailing list