Subject: Re: bug#10474: Building guile 2.x under mingw + msys

From: Andy Wingo <[email-masked]>
Date: Wed, 20 Feb 2013 22:57:18 +0100

Hi Eli,

On Wed 20 Feb 2013 20:14, Eli Zaretskii <[email-masked]> writes:

>> > 	Avoid compiler warnings on MS-Windows.
>> >
>> > 	* libguile/print.c (display_string_using_iconv): Cast 2nd arg of
>> > 	`iconv' to `const char **', to avoid compiler warnings.
>> >
>> > 	* libguile/ports.c (get_iconv_codepoint): Cast 2nd arg of `iconv'
>> > 	to `const char **', to avoid compiler warnings.
>> >
>> >
>> > --- libguile/print.c~0	2011-10-08 01:49:48.000000000 +0200
>> > +++ libguile/print.c	2012-01-15 15:10:51.450848400 +0200
>> > @@ -899,7 +899,7 @@ display_string_using_iconv (const void *
>> >        output = encoded_output;
>> >        output_left = sizeof (encoded_output);
>> >  
>> > -      done = iconv (pt->output_cd, &input, &input_left,
>> > +      done = iconv (pt->output_cd, (const char **)&input, &input_left,
>> >  		    &output, &output_left);
>> >  
>> >        output_len = sizeof (encoded_output) - output_left;
>> >
>> >
>> > --- libguile/ports.c~0	2011-10-08 01:49:48.000000000 +0200
>> > +++ libguile/ports.c	2012-01-15 15:11:11.856706600 +0200
>> > @@ -1305,7 +1305,7 @@ get_iconv_codepoint (SCM port, scm_t_wch
>> >        input_left = bytes_consumed + 1;
>> >        output_left = sizeof (utf8_buf);
>> >  
>> > -      done = iconv (pt->input_cd, &input, &input_left,
>> > +      done = iconv (pt->input_cd, (const char **)&input, &input_left,
>> >  		    &output, &output_left);
>> >        if (done == (size_t) -1)
>> >  	{
>> >
>> 
>> However iconv is specified
>> (http://pubs.opengroup.org/onlinepubs/009695399/functions/iconv.html) to
>> take a char** as the first argument.  Don't we end up using a GNU iconv
>> on mingw32 anyway?
>
> Yes, we do use GNU iconv.  However, the version of iconv.h (from GNU
> iconv version 1.13, I think) that I have declares the function like
> this:
>
>   extern size_t iconv (iconv_t cd, const char* * inbuf, size_t *inbytesleft, char* * outbuf, size_t *outbytesleft);

In these files, `input' is a char*.  So &input should be a char**.
Shouldn't that cast to const char** without a warning?

>> > 	* libguile/mkstemp.c (O_BINARY): Define for all platforms.
>> > 	(mkstemp): Open the temporary file in binary mode, so that
>> > 	compiled *.go files are written verbatim on MS-Windows.
>> 
>> Hummmmmmm.  It's true that the only user of mkstemp in Guile is the
>> compilation code.  OTOH it's a public interface, and this change might
>> affect someone.  OTOH MinGW is not well-served currently and probably
>> there are ~0 active users.  Any other thoughts here?
>
> An alternative would be to provide a public interface to switch an
> existing file descriptor to binary mode, and then use it from the code
> that calls mkstemp to output what will eventually become a *.go file.

You know, given that temporary files are for writing more than for
reading, it makes sense to just add the O_BINARY flag and be done with
it.  I'll do that.

>> > 	* libguile/net_db.c [HAVE_WINSOCK2_H]: Add !GNULIB_TEST_SOCKET to
>> > 	the condition, to include sys/socket.h and netdb.h when gnulib's
>> > 	socket module is being used.  Fixes compiler warnings and errors
>> > 	on MS-Windows.
>> >
>> > 	* libguile/socket.c: Likewise.
>> >
>> >
>> > --- libguile/net_db.c~0	2011-07-07 02:49:59.000000000 +0300
>> > +++ libguile/net_db.c	2012-01-15 16:22:56.366898100 +0200
>> > @@ -49,8 +49,11 @@
>> >  
>> >  #include <sys/types.h>
>> >  
>> > -#ifdef HAVE_WINSOCK2_H
>> > +#if HAVE_WINSOCK2_H && !GNULIB_TEST_SOCKET
>> >  #include <winsock2.h>
>> > +# if HAVE_WS2TCPIP_H
>> > +#  include <ws2tcpip.h>
>> > +# endif
>> >  #else
>> >  #include <sys/socket.h>
>> >  #include <netdb.h>
>> 
>> Surely we should just rely on Gnulib here and not include the winsock
>> headers.  In what condition would these headers be included otherwise?
>> A visual studio build or something?
>
> Sorry, I have no idea.  If relying on gnulib is OK for non-MinGW
> Windows platforms (if there are such), then it's fine with me.

I think for now this is the right thing.  It seems that even visual
studio builds are supported by gnulib, so the specs we code against
should either be posix or gnulib abstractions.  I have removed this #if
block.

Thanks,

Andy
-- 
http://wingolog.org/