Bug #5833: [mingw] trivial patch for thread.c build warning

• ruby -v changed from ruby 2.0.0dev (2011-12-31 trunk 34165) [i386-mingw32] to -

> Below is a trivial patch that fixes the warning on MinGW. The patch successfully builds and tests with MinGW and Windows SDK on Win7 32bit. When building on Arch Linux, the patched build gives the same `make test && make test-all` results (6 errors, 45 skips) as an unpatched build. In all other uses in `thread.c` (except for `rb_fd_rcopy`) the `rb_fd_max` macro generates a `size_t` object. From what I can tell, use cases are all unsigned int's. > > diff --git a/thread.c b/thread.c > index d9fe5506..0b48061 100644 > --- a/thread.c > +++ b/thread.c > @@ -2463,7 +2463,7 @@ rb_fd_init_copy(rb_fdset_t *dst, rb_fdset_t *src) >  static void >  rb_fd_rcopy(fd_set *dst, rb_fdset_t *src) >  { > -    int max = rb_fd_max(src); > +    size_t max = rb_fd_max(src); > >     /* we assume src is the result of select() with dst, so dst should be >      * larger or equal than src. */ I think rb_fd_max() should return int. Is there any possibility that fdset->fd_count overflow signed int? Moreover Windows fd_set::fd_count has u_int type if a documentation is correct (I saw http://msdn.microsoft.com/en-us/library/windows/desktop/ms737873(v=vs.85).aspx) and size_t is not an alias of u_int.

Redmine appears to have chopped off your feedback and questions...inserting your questions and my answers. > I think rb_fd_max() should return int. Is there any possibility that > fdset->fd_count overflow signed int? In real-world usage, I don't know with 100% certainty. But since the following returns a `u_int` on Windows https://github.com/ruby/ruby/blob/trunk/include/ruby/intern.h#L298 and this returns `FD_SETSIZE` (usually #defined) https://github.com/ruby/ruby/blob/trunk/include/ruby/intern.h#L314 overflow of signed int _might_ be a possibility. Why do you think `rb_fd_max` should return `int` rather than `unsigned int`? Is it because of this: https://github.com/ruby/ruby/blob/trunk/include/ruby/intern.h#L260 Do you think there is a possibility of returning a negative int (maybe an extension?) in a *valid* real-world scenario? > Moreover Windows fd_set::fd_count has u_int type if a documentation is > correct (I saw http://msdn.microsoft.com/en-us/library/windows/desktop/ms737873(v=vs.85).aspx) > and size_t is not an alias of u_int. For 32bit mingw64, headers appear to have `typedef unsigned int size_t` like lines 380-388 of: http://mingw-w64.svn.sourceforge.net/viewvc/mingw-w64/tags/v2.0.1/mingw-w64-headers/crt/_mingw.h.in?revision=4710&view=markup For 32bit mingw64, `fdset->fd_count` type is `u_int`: http://mingw-w64.svn.sourceforge.net/viewvc/mingw-w64/tags/v2.0.1/mingw-w64-headers/include/psdk_inc/_fd_types.h?revision=4710&view=markup with `typedef unsigned int u_int` like: http://mingw-w64.svn.sourceforge.net/viewvc/mingw-w64/tags/v2.0.1/mingw-w64-headers/crt/_bsd_types.h?revision=4710&view=markup And I see similar typedefs when searching my local mingw.org headers for `u_int` and `size_t`. In my Windows 7 SDK I see the following in `io.h` and `crtdefs.h`: #ifndef _SIZE_T_DEFINED #ifdef _WIN64 typedef unsigned __int64 size_t; #else typedef _W64 unsigned int size_t; #endif #define _SIZE_T_DEFINED #endif in which `_W64` is defined like i: #if !defined(_W64) #if !defined(__midl) && (defined(_X86_) || defined(_M_IX86)) #define _W64 __w64 #else #define _W64 #endif #endif

• Assignee set to akr (Akira Tanaka)

• Status changed from Open to Closed