On Fri, 2010-01-29 at 14:42 +0000, Vincent Sanders wrote:

> Added files

I've ignored everything in windows/

> Index: utils/utsname.h
> ===================================================================
> --- /dev/null 2009-11-01 16:06:14.000000000 +0000
> +++ utils/utsname.h   2010-01-29 12:09:20.000000000 +0000
> @@ -0,0 +1,39 @@
> +/*
> + * Copyright 2010 Vincent Sanders <[email protected]>
> + *
> + * This file is part of NetSurf, http://www.netsurf-browser.org/
> + *
> + * NetSurf is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; version 2 of the License.
> + *
> + * NetSurf is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program.  If not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +#ifndef _NETSURF_UTILS_UTSNAME_H_
> +#define _NETSURF_UTILS_UTSNAME_H_
> +
> +#ifdef _WIN32

I'd prefer to see #ifdef HAVE_UTSNAME or somesuch, which is then set or
otherwise in config.h. Otherwise, this file is fine.

> Changed files

All fine, except where noted.

> Index: utils/utils.c
> ===================================================================
> --- utils/utils.c     (revision 9918)
> +++ utils/utils.c     (working copy)
> @@ -463,3 +463,17 @@
>  }
>  
>  #endif
> +
> +#ifdef _WIN32

As per utsname.h, I'd prefer a platform-agnostic conditional here.

> Index: utils/filename.c
> ===================================================================
> --- utils/filename.c  (revision 9918)
> +++ utils/filename.c  (working copy)
> @@ -21,6 +21,11 @@
>   *
>   * A maximum of 2^24 files can be allocated at any point in time.
>   */
> +#ifdef _WIN32
> +#define ns_mkdir(dir, mode) mkdir((dir))
> +#else
> +#define ns_mkdir(dir, mode) mkdir((dir), (mode))
> +#endif

Can this be factored out into utils.[c/h] -- I fear that we'll end up
with duplication, otherwise. Also, I'd prefer a HAVE_ define, rather
than the platform-specific one.

> Index: desktop/netsurf.c
> ===================================================================
> --- desktop/netsurf.c (revision 9918)
> +++ desktop/netsurf.c (working copy)
> @@ -18,12 +18,18 @@
>   * along with this program.  If not, see <http://www.gnu.org/licenses/>.
>   */
>  
> +#ifdef _WIN32
> +#include <windows.h>
> +#include <shellapi.h>
> +#include "windows.h"
> +#endif
> +

Why do we need this in here? main() has been factored out to the
frontends now, right? Surely it can perform the appropriate platform
specific stuff there?

>  #include <locale.h>
>  #include <signal.h>
>  #include <stdbool.h>
>  #include <stdio.h>
>  #include <stdlib.h>
> -#include <sys/utsname.h>
> +#include "utils/utsname.h"
>  #include <libxml/encoding.h>
>  #include <libxml/globals.h>
>  #include <libxml/xmlversion.h>
> @@ -83,32 +89,42 @@
>  {
>       struct utsname utsname;
>  
> +#ifndef _WIN32
>       /* Ignore SIGPIPE - this is necessary as OpenSSL can generate these
>        * and the default action is to terminate the app. There's no easy
>        * way of determining the cause of the SIGPIPE (other than using
>        * sigaction() and some mechanism for getting the file descriptor
>        * out of libcurl). However, we expect nothing else to generate a
>        * SIGPIPE, anyway, so may as well just ignore them all. */
> +     
>       signal(SIGPIPE, SIG_IGN);
> +#endif

What's wrong with signal()? It's standard C.

>  #if !((defined(__SVR4) && defined(__sun)) || defined(__NetBSD__) || \
> -     defined(__OpenBSD__)) 
> +     defined(__OpenBSD__) || defined(_WIN32)) 
>       stdout = stderr;
>  #endif
>  
>       if ((argc > 1) && (argv[1][0] == '-') && (argv[1][1] == 'v') && 
> (argv[1][2] == 0)) {
> -         int argcmv;
> -         verbose_log = true;
> -         for (argcmv = 2; argcmv < argc; argcmv++) {
> -             argv[argcmv - 1] = argv[argcmv];
> -         }
> -         argc--;
> +             int argcmv;
> +             verbose_log = true;
> +             for (argcmv = 2; argcmv < argc; argcmv++) {
> +                     argv[argcmv - 1] = argv[argcmv];
> +             }
> +             argc--;
> +#ifdef _WIN32
> +             /* mwindows compile flag normally invalidates stdout unless 
> +                already redirected */
> +             if (_get_osfhandle(fileno(stdout)) == -1) {
> +                     AllocConsole();
> +                     freopen("CONOUT$", "w", stdout);
> +             }
> +#endif

I'd really rather this wasn't here.


J.


Reply via email to