Merge branch '024_msvc_squashed'

Conflicts:
	src/or/or.h
	 srcwin32/orconfig.h

Makefile.nmake

@@ -3,3 +3,13 @@ all:
 	$(MAKE) /F Makefile.nmake
 	cd ../../src/or
 	$(MAKE) /F Makefile.nmake
+	cd ../../src/test
+	$(MAKE) /F Makefile.nmake
+
+clean:
+	cd src/common
+	$(MAKE) /F Makefile.nmake clean
+	cd ../../src/or
+	$(MAKE) /F Makefile.nmake clean
+	cd ../../src/test
+	$(MAKE) /F Makefile.nmake clean

changes/7304

@@ -0,0 +1,7 @@
+  o Minor bugfixes:
+    - Replace all calls to snprintf outside of src/ext with tor_snprintf.
+      Fix for bug 7304.
+  o Code simplification and refactoring:
+    - Remove the #define to replace snprintf with _snprintf on windows;
+      they have different semantics, and all of our callers should be
+      using tor_snprintf anyway. Partial fix for bug 7304.

changes/build_with_msvc

@@ -0,0 +1,4 @@
+  o Features (portability):
+    - Build correctly with MSVC and Makefile.nmake. This resolves
+      numerous bugs and fixes reported by ultramage, including 7305,
+      7308, 7309, 7310, 7312, 7313, 7315, 7316, and 7669.

src/common/Makefile.nmake

@@ -1,6 +1,6 @@
 all: libor.lib libor-crypto.lib libor-event.lib
 
-CFLAGS = /I ..\win32 /I ..\..\..\build-alpha\include
+CFLAGS = /I ..\win32 /I ..\..\..\build-alpha\include /I ..\ext
 
 LIBOR_OBJECTS = address.obj compat.obj container.obj di_ops.obj \
 	log.obj memarea.obj mempool.obj procmon.obj util.obj \
@@ -18,3 +18,6 @@ libor-crypto.lib: $(LIBOR_CRYPTO_OBJECTS)
 
 libor-event.lib:  $(LIBOR_EVENT_OBJECTS)
 	lib $(LIBOR_EVENT_OBJECTS) /out:libor-event.lib
+
+clean:
+	del *.obj *.lib libor*.lib

src/common/compat.h

@@ -60,7 +60,6 @@
 #include <io.h>
 #include <math.h>
 #include <projects.h>
-#define snprintf _snprintf
 /* this is not exported as W .... */
 #define SHGetPathFromIDListW SHGetPathFromIDList
 /* wcecompat has vasprintf */
@@ -133,6 +132,17 @@ extern INLINE double U64_TO_DBL(uint64_t x) {
 #define DBL_TO_U64(x) ((uint64_t) (x))
 #endif
 
+#if defined(_MSC_VER)
+/* XXXX024 we should instead have a more general check for "Is enum signed?"*/
+#define ENUM_BF(t) unsigned
+#else
+/** Wrapper for having a bitfield of an enumerated type. Where possible, we
+ * just use the enumerated type (so the compiler can help us and notice
+ * problems), but if enumerated types are unsigned, we must use unsigned,
+ * so that the loss of precision doesn't make large values negative. */
+#define ENUM_BF(t) t
+#endif
+
 /* GCC has several useful attributes. */
 #if defined(__GNUC__) && __GNUC__ >= 3
 #define ATTR_NORETURN __attribute__((noreturn))

src/common/crypto.c

@@ -1505,7 +1505,7 @@ struct crypto_digest_t {
     SHA256_CTX sha2; /**< state for SHA256 */
   } d; /**< State for the digest we're using.  Only one member of the
         * union is usable, depending on the value of <b>algorithm</b>. */
-  digest_algorithm_t algorithm : 8; /**< Which algorithm is in use? */
+  ENUM_BF(digest_algorithm_t) algorithm : 8; /**< Which algorithm is in use? */
 };
 
 /** Allocate and return a new digest object to compute SHA1 digests.

src/common/torint.h

@@ -214,16 +214,20 @@ typedef int32_t ssize_t;
 #if (SIZEOF_VOID_P > 4 && SIZEOF_VOID_P <= 8)
 #ifndef HAVE_INTPTR_T
 typedef int64_t intptr_t;
+#define SIZEOF_INTPTR_T 8
 #endif
 #ifndef HAVE_UINTPTR_T
 typedef uint64_t uintptr_t;
+#define SIZEOF_UINTPTR_T 8
 #endif
 #elif (SIZEOF_VOID_P > 2 && SIZEOF_VOID_P <= 4)
 #ifndef HAVE_INTPTR_T
 typedef int32_t intptr_t;
+#define SIZEOF_INTPTR_T 4
 #endif
 #ifndef HAVE_UINTPTR_T
 typedef uint32_t uintptr_t;
+#define SIZEOF_UINTPTR_T 4
 #endif
 #else
 #error "void * is either >8 bytes or <= 2.  In either case, I am confused."

src/common/tortls.c

@@ -147,6 +147,12 @@ typedef struct tor_tls_context_t {
 
 #define TOR_TLS_MAGIC 0x71571571
 
+typedef enum {
+    TOR_TLS_ST_HANDSHAKE, TOR_TLS_ST_OPEN, TOR_TLS_ST_GOTCLOSE,
+    TOR_TLS_ST_SENTCLOSE, TOR_TLS_ST_CLOSED, TOR_TLS_ST_RENEGOTIATE,
+    TOR_TLS_ST_BUFFEREVENT
+} tor_tls_state_t;
+
 /** Holds a SSL object and its associated data.  Members are only
  * accessed from within tortls.c.
  */
@@ -156,12 +162,9 @@ struct tor_tls_t {
   SSL *ssl; /**< An OpenSSL SSL object. */
   int socket; /**< The underlying file descriptor for this TLS connection. */
   char *address; /**< An address to log when describing this connection. */
-  enum {
-    TOR_TLS_ST_HANDSHAKE, TOR_TLS_ST_OPEN, TOR_TLS_ST_GOTCLOSE,
-    TOR_TLS_ST_SENTCLOSE, TOR_TLS_ST_CLOSED, TOR_TLS_ST_RENEGOTIATE,
-    TOR_TLS_ST_BUFFEREVENT
-  } state : 3; /**< The current SSL state, depending on which operations have
-                * completed successfully. */
+  ENUM_BF(tor_tls_state_t) state : 3; /**< The current SSL state,
+                                       * depending on which operations
+                                       * have completed successfully. */
   unsigned int isServer:1; /**< True iff this is a server-side connection */
   unsigned int wasV2Handshake:1; /**< True iff the original handshake for
                                   * this connection used the updated version

src/common/util.c

@@ -4919,7 +4919,8 @@ tor_check_port_forwarding(const char *filename,
     status = tor_spawn_background(filename, argv, NULL, &child_handle);
 #endif
 
-    tor_free(argv);
+    tor_free_((void*)argv);
+    argv=NULL;
 
     if (PROCESS_STATUS_ERROR == status) {
       log_warn(LD_GENERAL, "Failed to start port forwarding helper %s",

src/or/Makefile.nmake

@@ -1,28 +1,70 @@
 all: tor.exe
 
-CFLAGS = /I ..\win32 /I ..\..\..\build-alpha\include /I ..\common
+CFLAGS = /I ..\win32 /I ..\..\..\build-alpha\include /I ..\common \
+    /I ..\ext
 
-LIBS = ..\..\..\build-alpha\lib\libevent.a \
- ..\..\..\build-alpha\lib\libcrypto.a \
- ..\..\..\build-alpha\lib\libssl.a \
- ..\..\..\build-alpha\lib\libz.a \
+LIBS = ..\..\..\build-alpha\lib\libevent.lib \
+ ..\..\..\build-alpha\lib\libcrypto.lib \
+ ..\..\..\build-alpha\lib\libssl.lib \
+ ..\..\..\build-alpha\lib\libz.lib \
  ws2_32.lib advapi32.lib shell32.lib
 
-LIBTOR_OBJECTS = buffers.obj channel.obj channeltls.obj circuitbuild.obj \
-	circuitlist.obj circuitmux.obj circuitmux_ewma.obj circuituse.obj \
-	command.obj config.obj connection.obj connection_edge.obj \
-	connection_or.obj control.obj cpuworker.obj directory.obj \
-	dirserv.obj dirvote.obj dns.obj dnsserv.obj geoip.obj hibernate.obj \
-	main.obj microdesc.obj networkstatus.obj nodelist.obj onion.obj \
-	policies.obj reasons.obj relay.obj rendclient.obj rendcommon.obj \
-	rendmid.obj rendservice.obj rephist.obj router.obj routerlist.obj \
-	routerparse.obj status.obj config_codedigest.obj ntmain.obj
+LIBTOR_OBJECTS = \
+  addressmap.obj \
+  buffers.obj \
+  channel.obj \
+  channeltls.obj \
+  circuitbuild.obj \
+  circuitlist.obj \
+  circuitmux.obj \
+  circuitmux_ewma.obj \
+  circuitstats.obj \
+  circuituse.obj \
+  command.obj \
+  config.obj \
+  config_codedigest.obj \
+  confparse.obj \
+  connection.obj \
+  connection_edge.obj \
+  connection_or.obj \
+  control.obj \
+  cpuworker.obj \
+  directory.obj \
+  dirserv.obj \
+  dirvote.obj \
+  dns.obj \
+  dnsserv.obj \
+  entrynodes.obj \
+  geoip.obj \
+  hibernate.obj \
+  main.obj \
+  microdesc.obj \
+  networkstatus.obj \
+  nodelist.obj \
+  ntmain.obj \
+  onion.obj \
+  policies.obj \
+  reasons.obj \
+  relay.obj \
+  rendclient.obj \
+  rendcommon.obj \
+  rendmid.obj \
+  rendservice.obj \
+  rephist.obj \
+  replaycache.obj \
+  router.obj \
+  routerlist.obj \
+  routerparse.obj \
+  routerset.obj \
+  statefile.obj \
+  status.obj \
+  transports.obj
 
 libtor.lib: $(LIBTOR_OBJECTS)
-	lib $(LIBTOR_OBJECTS) /out:libtor.lib
+	lib $(LIBTOR_OBJECTS) /out:$@
 
 tor.exe: libtor.lib tor_main.obj
-	$(CC) $(CFLAGS) $(LIBS) libtor.lib ..\common\*.lib tor_main.obj
+	$(CC) $(CFLAGS) $(LIBS) libtor.lib ..\common\*.lib tor_main.obj /Fe$@
 
 clean:
 	del $(LIBTOR_OBJECTS) *.lib tor.exe

src/or/addressmap.c

@@ -45,7 +45,7 @@
 typedef struct {
   char *new_address;
   time_t expires;
-  addressmap_entry_source_t source:3;
+  ENUM_BF(addressmap_entry_source_t) source:3;
   unsigned src_wildcard:1;
   unsigned dst_wildcard:1;
   short num_resolve_failures;

src/or/channel.h

@@ -142,7 +142,7 @@ struct channel_s {
    * When we send CREATE cells along this connection, which half of the
    * space should we use?
    */
-  circ_id_type_t circ_id_type:2;
+  ENUM_BF(circ_id_type_t) circ_id_type:2;
   /*
    * Which circ_id do we try to use next on this connection?  This is
    * always in the range 0..1<<15-1.

src/or/or.h

@@ -81,7 +81,6 @@
 #include <process.h>
 #include <direct.h>
 #include <windows.h>
-#define snprintf _snprintf
 #endif
 
 #ifdef USE_BUFFEREVENTS
@@ -1592,6 +1591,13 @@ typedef struct entry_connection_t {
 
 } entry_connection_t;
 
+typedef enum {
+    DIR_SPOOL_NONE=0, DIR_SPOOL_SERVER_BY_DIGEST, DIR_SPOOL_SERVER_BY_FP,
+    DIR_SPOOL_EXTRA_BY_DIGEST, DIR_SPOOL_EXTRA_BY_FP,
+    DIR_SPOOL_CACHED_DIR, DIR_SPOOL_NETWORKSTATUS,
+    DIR_SPOOL_MICRODESC, /* NOTE: if we add another entry, add another bit. */
+} dir_spool_source_t;
+
 /** Subtype of connection_t for an "directory connection" -- that is, an HTTP
  * connection to retrieve or serve directory material. */
 typedef struct dir_connection_t {
@@ -1610,12 +1616,8 @@ typedef struct dir_connection_t {
    * "spooling" of directory material to the outbuf.  Otherwise, we'd have
    * to append everything to the outbuf in one enormous chunk. */
   /** What exactly are we spooling right now? */
-  enum {
-    DIR_SPOOL_NONE=0, DIR_SPOOL_SERVER_BY_DIGEST, DIR_SPOOL_SERVER_BY_FP,
-    DIR_SPOOL_EXTRA_BY_DIGEST, DIR_SPOOL_EXTRA_BY_FP,
-    DIR_SPOOL_CACHED_DIR, DIR_SPOOL_NETWORKSTATUS,
-    DIR_SPOOL_MICRODESC, /* NOTE: if we add another entry, add another bit. */
-  } dir_spool_src : 3;
+  ENUM_BF(dir_spool_source_t)  dir_spool_src : 3;
+
   /** If we're fetching descriptors, what router purpose shall we assign
    * to them? */
   uint8_t router_purpose;
@@ -1791,7 +1793,8 @@ typedef enum {
 /** A reference-counted address policy rule. */
 typedef struct addr_policy_t {
   int refcnt; /**< Reference count */
-  addr_policy_action_t policy_type:2;/**< What to do when the policy matches.*/
+  /** What to do when the policy matches.*/
+  ENUM_BF(addr_policy_action_t) policy_type:2;
   unsigned int is_private:1; /**< True iff this is the pseudo-address,
                               * "private". */
   unsigned int is_canonical:1; /**< True iff this policy is the canonical
@@ -1859,7 +1862,7 @@ typedef struct download_status_t {
                            * again? */
   uint8_t n_download_failures; /**< Number of failures trying to download the
                                 * most recent descriptor. */
-  download_schedule_t schedule : 8;
+  ENUM_BF(download_schedule_t) schedule : 8;
 } download_status_t;
 
 /** If n_download_failures is this high, the download can never happen. */
@@ -2128,7 +2131,7 @@ typedef struct microdesc_t {
    */
   time_t last_listed;
   /** Where is this microdescriptor currently stored? */
-  saved_location_t saved_location : 3;
+  ENUM_BF(saved_location_t) saved_location : 3;
   /** If true, do not attempt to cache this microdescriptor on disk. */
   unsigned int no_save : 1;
   /** If true, this microdesc has an entry in the microdesc_map */
@@ -2378,8 +2381,8 @@ typedef enum {
 /** A common structure to hold a v3 network status vote, or a v3 network
  * status consensus. */
 typedef struct networkstatus_t {
-  networkstatus_type_t type : 8; /**< Vote, consensus, or opinion? */
-  consensus_flavor_t flavor : 8; /**< If a consensus, what kind? */
+  ENUM_BF(networkstatus_type_t) type : 8; /**< Vote, consensus, or opinion? */
+  ENUM_BF(consensus_flavor_t) flavor : 8; /**< If a consensus, what kind? */
   time_t published; /**< Vote only: Time when vote was written. */
   time_t valid_after; /**< Time after which this vote or consensus applies. */
   time_t fresh_until; /**< Time before which this is the most recent vote or
@@ -2887,7 +2890,7 @@ typedef struct origin_circuit_t {
 
   /** Kludge to help us prevent the warn in bug #6475 and eventually
    * debug why we are not seeing first hops in some cases. */
-  path_state_t path_state : 3;
+  ENUM_BF(path_state_t) path_state : 3;
 
   /** For path probing. Store the temporary probe stream ID
    * for response comparison */

src/or/routerparse.c

@@ -3844,7 +3844,7 @@ get_next_token(memarea_t *area,
   if ((size_t)(eol-next) != 9+obname_len+5 ||
       strcmp_len(next+9, tok->object_type, obname_len) ||
       strcmp_len(eol-5, "-----", 5)) {
-    snprintf(ebuf, sizeof(ebuf), "Malformed object: mismatched end tag %s",
+    tor_snprintf(ebuf, sizeof(ebuf), "Malformed object: mismatched end tag %s",
              tok->object_type);
     ebuf[sizeof(ebuf)-1] = '\0';
     RET_ERR(ebuf);

src/test/Makefile.nmake

@@ -1,6 +1,7 @@
-all: test.exe
+all: test.exe test-child.exe bench.exe
 
-CFLAGS = /I ..\win32 /I ..\..\..\build-alpha\include /I ..\common /I ..\or
+CFLAGS = /I ..\win32 /I ..\..\..\build-alpha\include /I ..\common /I ..\or \
+    /I ..\ext
 
 LIBS = ..\..\..\build-alpha\lib\libevent.lib \
  ..\..\..\build-alpha\lib\libcrypto.lib \
@@ -11,10 +12,20 @@ LIBS = ..\..\..\build-alpha\lib\libevent.lib \
 
 TEST_OBJECTS = test.obj test_addr.obj test_containers.obj \
 	test_crypto.obj test_data.obj test_dir.obj test_microdesc.obj \
-	test_pt.obj test_util.obj test_config.obj tinytest.obj
+	test_pt.obj test_util.obj test_config.obj test_cell_formats.obj \
+	test_replay.obj test_introduce.obj tinytest.obj
+
+tinytest.obj: ..\ext\tinytest.c
+	$(CC) $(CFLAGS) /D snprintf=_snprintf /c ..\ext\tinytest.c
 
 test.exe: $(TEST_OBJECTS)
-	$(CC) $(CFLAGS) $(LIBS) ..\common\*.lib $(TEST_OBJECTS)
+	$(CC) $(CFLAGS) $(LIBS) ..\common\*.lib $(TEST_OBJECTS) /Fe$@
+
+bench.exe: bench.obj
+	$(CC) $(CFLAGS) bench.obj $(LIBS) ..\common\*.lib /Fe$@
+
+test-child.exe: test-child.obj
+	$(CC) $(CFLAGS) test-child.obj /Fe$@
 
 clean:
-	del $(TEST_OBJECTS) *.lib test.exe
+	del *.obj *.lib test.exe bench.exe test-child.exe

src/win32/orconfig.h

@@ -247,5 +247,6 @@
 #define RSHIFT_DOES_SIGN_EXTEND
 #define FLEXIBLE_ARRAY_MEMBER 0
 #define HAVE_EVENT2_EVENT_H
-#define HAVE_EVENT_BASE_LOOPEXIT
 #define SHARE_DATADIR ""
+#define HAVE_EVENT2_DNS_H
+#define HAVE_EVENT_BASE_LOOPEXIT