Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(1341)

Issue 342870043: Visual Studio conditional code for core module (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
8 months, 2 weeks ago by ammo6818-vandals.uidaho.edu
Modified:
6 months, 1 week ago
Reviewers:
Peter Barnes
Visibility:
Public.

Description

Visual Studio conditional code for core module

Patch Set 1 #

Patch Set 2 : Updates for review comments #

Patch Set 3 : Incorporate changes for ATTRIBUTE_* macros and latest module changes #

Patch Set 4 : Corrections #

Total comments: 57

Patch Set 5 : Updates to resolve review comments #

Unified diffs Side-by-side diffs Delta from patch set Stats (+1624 lines, -513 lines) Patch
M src/core/examples/main-callback.cc View 1 2 3 4 2 chunks +5 lines, -0 lines 0 comments Download
M src/core/examples/main-ptr.cc View 1 2 3 4 2 chunks +5 lines, -0 lines 0 comments Download
M src/core/examples/main-random-variable.cc View 1 2 3 4 2 chunks +5 lines, -0 lines 0 comments Download
M src/core/examples/main-random-variable-stream.cc View 1 2 3 4 2 chunks +5 lines, -0 lines 0 comments Download
M src/core/helper/event-garbage-collector.h View 1 2 3 4 2 chunks +6 lines, -1 line 0 comments Download
M src/core/helper/random-variable-stream-helper.h View 1 2 3 4 2 chunks +5 lines, -1 line 0 comments Download
M src/core/model/assert.h View 1 2 3 4 3 chunks +4 lines, -3 lines 0 comments Download
M src/core/model/attribute.h View 1 2 3 4 7 chunks +10 lines, -6 lines 0 comments Download
M src/core/model/attribute-construction-list.h View 1 2 3 4 2 chunks +5 lines, -1 line 0 comments Download
M src/core/model/attribute-helper.h View 1 2 3 4 11 chunks +64 lines, -2 lines 0 comments Download
M src/core/model/boolean.h View 1 2 3 4 3 chunks +7 lines, -3 lines 0 comments Download
M src/core/model/boolean.cc View 1 2 3 4 1 chunk +1 line, -1 line 0 comments Download
M src/core/model/cairo-wideint.c View 1 2 3 4 4 chunks +4 lines, -4 lines 0 comments Download
M src/core/model/cairo-wideint-private.h View 1 2 3 4 5 chunks +8 lines, -8 lines 0 comments Download
M src/core/model/calendar-scheduler.h View 1 2 3 4 2 chunks +5 lines, -1 line 0 comments Download
M src/core/model/callback.h View 1 2 3 4 4 chunks +8 lines, -4 lines 0 comments Download
M src/core/model/callback.cc View 1 2 3 4 2 chunks +3 lines, -3 lines 0 comments Download
M src/core/model/command-line.h View 1 2 3 4 7 chunks +19 lines, -6 lines 0 comments Download
M src/core/model/command-line.cc View 1 2 3 4 2 chunks +8 lines, -0 lines 0 comments Download
M src/core/model/config.h View 1 2 3 4 14 chunks +21 lines, -16 lines 0 comments Download
M src/core/model/default-simulator-impl.h View 1 2 3 4 2 chunks +5 lines, -1 line 0 comments Download
M src/core/model/des-metrics.h View 1 2 3 4 2 chunks +5 lines, -1 line 0 comments Download
M src/core/model/double.h View 1 2 3 4 3 chunks +7 lines, -3 lines 0 comments Download
M src/core/model/double.cc View 1 2 3 4 1 chunk +1 line, -1 line 0 comments Download
M src/core/model/enum.h View 1 2 3 4 4 chunks +7 lines, -3 lines 0 comments Download
M src/core/model/event-id.h View 1 2 3 4 2 chunks +5 lines, -1 line 0 comments Download
M src/core/model/event-impl.h View 1 2 3 4 2 chunks +5 lines, -1 line 0 comments Download
M src/core/model/fatal-impl.h View 1 2 3 4 4 chunks +7 lines, -3 lines 0 comments Download
M src/core/model/fatal-impl.cc View 1 2 3 4 2 chunks +14 lines, -4 lines 0 comments Download
A src/core/model/fd-reader.h View 1 2 3 4 1 chunk +144 lines, -0 lines 0 comments Download
M src/core/model/global-value.h View 1 2 3 4 2 chunks +5 lines, -1 line 0 comments Download
M src/core/model/hash.h View 1 2 3 4 2 chunks +5 lines, -1 line 0 comments Download
M src/core/model/hash-fnv.h View 1 2 3 4 2 chunks +5 lines, -1 line 0 comments Download
M src/core/model/hash-function.h View 1 2 3 4 4 chunks +7 lines, -3 lines 0 comments Download
M src/core/model/hash-murmur3.h View 1 2 3 4 2 chunks +5 lines, -1 line 0 comments Download
M src/core/model/heap-scheduler.h View 1 2 3 4 2 chunks +5 lines, -1 line 0 comments Download
M src/core/model/int64x64.h View 1 2 3 4 3 chunks +9 lines, -5 lines 0 comments Download
M src/core/model/int64x64-128.h View 1 2 3 4 2 chunks +5 lines, -1 line 0 comments Download
M src/core/model/int64x64-cairo.h View 1 2 3 4 3 chunks +14 lines, -10 lines 0 comments Download
M src/core/model/int64x64-double.h View 1 2 3 4 2 chunks +6 lines, -1 line 0 comments Download
M src/core/model/integer.h View 1 2 3 4 3 chunks +7 lines, -3 lines 0 comments Download
M src/core/model/integer.cc View 1 2 3 4 1 chunk +1 line, -1 line 0 comments Download
M src/core/model/list-scheduler.h View 1 2 3 4 2 chunks +5 lines, -1 line 0 comments Download
M src/core/model/log.h View 1 2 3 4 12 chunks +20 lines, -16 lines 0 comments Download
M src/core/model/log-macros-enabled.h View 1 2 3 4 3 chunks +3 lines, -5 lines 0 comments Download
M src/core/model/make-event.h View 1 2 3 4 2 chunks +5 lines, -1 line 0 comments Download
M src/core/model/map-scheduler.h View 1 2 3 4 2 chunks +5 lines, -1 line 0 comments Download
M src/core/model/names.h View 1 2 3 4 2 chunks +5 lines, -1 line 0 comments Download
M src/core/model/names.cc View 1 chunk +2 lines, -0 lines 0 comments Download
M src/core/model/non-copyable.h View 1 2 3 4 2 chunks +5 lines, -1 line 0 comments Download
A src/core/model/ns3-export.h View 1 2 3 4 1 chunk +33 lines, -0 lines 0 comments Download
A src/core/model/ns3-module.h View 1 2 3 4 1 chunk +69 lines, -0 lines 0 comments Download
M src/core/model/nstime.h View 1 2 3 4 8 chunks +12 lines, -8 lines 0 comments Download
M src/core/model/object.h View 1 2 3 4 3 chunks +6 lines, -2 lines 0 comments Download
M src/core/model/object-base.h View 1 2 3 4 3 chunks +26 lines, -1 line 0 comments Download
M src/core/model/object-factory.h View 1 2 3 4 6 chunks +10 lines, -6 lines 0 comments Download
M src/core/model/object-factory.cc View 1 2 1 chunk +1 line, -1 line 0 comments Download
M src/core/model/object-ptr-container.h View 1 2 3 4 5 chunks +8 lines, -4 lines 0 comments Download
M src/core/model/pointer.h View 1 2 3 4 4 chunks +7 lines, -3 lines 0 comments Download
M src/core/model/random-variable-stream.h View 1 2 3 4 18 chunks +21 lines, -17 lines 0 comments Download
M src/core/model/realtime-simulator-impl.h View 1 2 3 4 2 chunks +5 lines, -1 line 0 comments Download
M src/core/model/ref-count-base.h View 1 2 3 4 2 chunks +5 lines, -1 line 0 comments Download
M src/core/model/rng-seed-manager.h View 1 2 3 4 2 chunks +5 lines, -1 line 0 comments Download
M src/core/model/rng-stream.h View 1 2 3 4 2 chunks +5 lines, -1 line 0 comments Download
M src/core/model/scheduler.h View 1 2 3 4 2 chunks +5 lines, -1 line 0 comments Download
M src/core/model/simulator.h View 1 2 3 4 3 chunks +6 lines, -2 lines 0 comments Download
M src/core/model/simulator-impl.h View 1 2 3 4 2 chunks +5 lines, -1 line 0 comments Download
M src/core/model/string.h View 1 2 3 4 2 chunks +7 lines, -3 lines 0 comments Download
M src/core/model/string.cc View 1 2 3 4 1 chunk +2 lines, -2 lines 0 comments Download
M src/core/model/synchronizer.h View 1 2 3 4 2 chunks +5 lines, -1 line 0 comments Download
M src/core/model/system-condition.h View 1 2 3 4 2 chunks +5 lines, -1 line 0 comments Download
M src/core/model/system-mutex.h View 1 2 3 4 3 chunks +6 lines, -2 lines 0 comments Download
M src/core/model/system-path.h View 1 2 3 4 8 chunks +43 lines, -10 lines 0 comments Download
M src/core/model/system-path.cc View 1 2 3 4 14 chunks +96 lines, -21 lines 0 comments Download
M src/core/model/system-thread.h View 1 2 3 4 2 chunks +5 lines, -1 line 0 comments Download
M src/core/model/system-thread.cc View 1 chunk +2 lines, -0 lines 0 comments Download
M src/core/model/system-wall-clock-ms.h View 1 2 3 4 2 chunks +5 lines, -1 line 0 comments Download
M src/core/model/test.h View 1 2 3 4 6 chunks +9 lines, -5 lines 0 comments Download
M src/core/model/test.cc View 1 2 3 4 6 chunks +21 lines, -62 lines 0 comments Download
M src/core/model/time.cc View 1 2 3 4 1 chunk +1 line, -1 line 0 comments Download
M src/core/model/timer.h View 1 2 3 4 2 chunks +5 lines, -1 line 0 comments Download
M src/core/model/timer-impl.h View 1 2 3 4 2 chunks +5 lines, -1 line 0 comments Download
M src/core/model/trace-source-accessor.h View 1 2 3 4 2 chunks +5 lines, -1 line 0 comments Download
M src/core/model/traced-value.h View 2 chunks +8 lines, -0 lines 0 comments Download
M src/core/model/type-id.h View 1 2 3 4 12 chunks +19 lines, -10 lines 0 comments Download
M src/core/model/type-id.cc View 1 2 3 4 9 chunks +31 lines, -13 lines 0 comments Download
M src/core/model/type-name.h View 1 2 3 4 2 chunks +14 lines, -10 lines 0 comments Download
M src/core/model/uinteger.h View 1 2 3 4 3 chunks +7 lines, -3 lines 0 comments Download
M src/core/model/uinteger.cc View 1 2 3 4 1 chunk +1 line, -1 line 0 comments Download
R src/core/model/unix-fd-reader.h View 1 2 3 4 1 chunk +0 lines, -140 lines 0 comments Download
M src/core/model/unix-fd-reader.cc View 1 2 3 4 1 chunk +1 line, -1 line 0 comments Download
M src/core/model/unix-system-condition.cc View 1 2 3 4 1 chunk +4 lines, -0 lines 0 comments Download
M src/core/model/valgrind.h View 1 2 3 4 3 chunks +14 lines, -2 lines 0 comments Download
M src/core/model/vector.h View 1 2 3 4 16 chunks +77 lines, -28 lines 0 comments Download
M src/core/model/vector.cc View 1 2 3 4 1 chunk +2 lines, -2 lines 0 comments Download
M src/core/model/wall-clock-synchronizer.h View 1 2 3 4 3 chunks +77 lines, -1 line 0 comments Download
M src/core/model/wall-clock-synchronizer.cc View 1 2 3 4 2 chunks +6 lines, -0 lines 0 comments Download
M src/core/model/watchdog.h View 1 2 3 4 2 chunks +5 lines, -1 line 0 comments Download
A src/core/model/win32-fd-reader.cc View 1 2 3 4 1 chunk +222 lines, -0 lines 0 comments Download
M src/core/model/win32-system-wall-clock-ms.cc View 1 2 3 4 2 chunks +3 lines, -1 line 0 comments Download
A src/core/model/win32-time.h View 1 2 3 4 1 chunk +83 lines, -0 lines 0 comments Download
A src/core/model/win32-time.cc View 1 2 3 4 1 chunk +62 lines, -0 lines 0 comments Download
M src/core/test/int64x64-test-suite.cc View 1 2 3 4 1 chunk +9 lines, -0 lines 0 comments Download
M src/core/wscript View 1 2 3 4 2 chunks +3 lines, -1 line 0 comments Download
M src/fd-net-device/model/fd-net-device.h View 1 2 3 4 1 chunk +1 line, -1 line 0 comments Download
M src/tap-bridge/model/tap-bridge.h View 1 2 3 4 1 chunk +1 line, -1 line 0 comments Download
M src/tap-bridge/model/tap-bridge.cc View 1 2 3 4 1 chunk +1 line, -1 line 0 comments Download

Messages

Total messages: 7
Peter Barnes
Where does _CORE get defined, to toggle between building core vs. referencing core?
8 months, 2 weeks ago (2018-03-30 05:11:18 UTC) #1
ammo6818-vandals.uidaho.edu
On 2018/03/30 05:11:18, Peter Barnes wrote: > Where does _CORE get defined, to toggle between ...
8 months, 2 weeks ago (2018-03-30 23:59:04 UTC) #2
ammo6818-vandals.uidaho.edu
Updated patch set uploaded to address review comments.
8 months, 1 week ago (2018-04-05 17:47:22 UTC) #3
ammo6818-vandals.uidaho.edu
Changes incorporated for ATTRIBUTE_* macros to address review comments. Latest module changes alos incorporated.
8 months, 1 week ago (2018-04-06 19:10:11 UTC) #4
ammo6818-vandals.uidaho.edu
Please let me know if you have any additional review comments I need to address.
7 months, 4 weeks ago (2018-04-16 15:14:09 UTC) #5
Peter Barnes
Hello Robert, Here are detailed comments on peripheral issues, and you may have seen some ...
7 months, 3 weeks ago (2018-04-17 00:04:11 UTC) #6
ammo6818-vandals.uidaho.edu
7 months, 1 week ago (2018-05-02 05:06:32 UTC) #7
I have posted an update based upon your review comments.  The  update includes
resolution of your comments for the phase 2 changes to core (and the other
affected modules) and phase 3 changes to core based upon your alternate
proposal.

The one thing that wont work from your alternate proposal is auto generation of
the ordinal numbers for the modules.  There isnt the ability to do something
equivalent to ./waf configure as a prebuild step and if this file is auto
generated it will cause a re compile of all files any time a build is performed.

I dont think there is an issue of statically defined ordinals for the ns-3 core
modules.  If modules are dropped from ns-3 core the ordinals are not reused.

The only issue would be contrib module conflicts with other contrib modules. 
But that issue would not be unique to ordinals, the same problem would exist for
header and source file names.

I really dont have a better alternative for this issue.

https://codereview.appspot.com/342870043/diff/60001/src/core/examples/main-ca...
File src/core/examples/main-callback.cc (right):

https://codereview.appspot.com/342870043/diff/60001/src/core/examples/main-ca...
src/core/examples/main-callback.cc:22: #include "ns3/command-line.h"
On 2018/04/17 00:04:12, Peter Barnes wrote:
> Also need to add this to each program, at the top of main():
> 
> CommandLine cmd;
> cmd.Parse (argc, arg);

Changed as noted.

https://codereview.appspot.com/342870043/diff/60001/src/core/examples/main-te...
File src/core/examples/main-test-sync.cc (right):

https://codereview.appspot.com/342870043/diff/60001/src/core/examples/main-te...
src/core/examples/main-test-sync.cc:30: #ifndef _WIN32
On 2018/04/17 00:04:12, Peter Barnes wrote:
> I've modified this file to use std::this_thread::sleep_for
> (std::chrono::seconds(1));  shouldn't need any Win-specific code.

Ok

https://codereview.appspot.com/342870043/diff/60001/src/core/model/attribute-...
File src/core/model/attribute-helper.h (right):

https://codereview.appspot.com/342870043/diff/60001/src/core/model/attribute-...
src/core/model/attribute-helper.h:51: *     - ATTRIBUTE_HELPER_CPP(type,lib) for
self contained programs
On 2018/04/17 00:04:12, Peter Barnes wrote:
> Need to clarify what's a library, and what's a self-contained program.  Why
> can't these be unified?

Changed to:

 * There are two kinds of helper macros:
 *   -# The simple macros.
 *     - ATTRIBUTE_HELPER_HEADER_LIB(type,lib) for classes that can be used by
other modules
 *     - ATTRIBUTE_HELPER_CPP_LIB(type,lib) for classes that can be used by
other modules
 *       or
 *     - ATTRIBUTE_HELPER_HEADER(type,lib) for classes that are only used in the
this executable unit
 *     - ATTRIBUTE_HELPER_CPP(type,lib) for classes that are only used in the
this executable unit
 *   -# The more complex macros.
 *

See attribute-test-suite.cc for an example of the latter.

https://codereview.appspot.com/342870043/diff/60001/src/core/model/attribute-...
src/core/model/attribute-helper.h:213: #define
ATTRIBUTE_VALUE_DEFINE_WITH_NAME(type,name)                     \
On 2018/04/17 00:04:12, Peter Barnes wrote:
> Can't this be implemented as
> 
> #define ATTRIBUTE_VALUE_DEFINE_WITH_NAME(type,name)                     \
>     ATTRIBUTE_VALUE_DEFINE_WITH_NAME_LIB(type,name,)
> 
> Then we wouldn't need to duplicate this macro body.

No,there is a subtle difference on line 191.

https://codereview.appspot.com/342870043/diff/60001/src/core/model/attribute-...
src/core/model/attribute-helper.h:319:
ATTRIBUTE_VALUE_IMPLEMENT_WITH_NAME(type,name)
On 2018/04/17 00:04:12, Peter Barnes wrote:
> All the implementation macros discard the lib argument.  If so, why add the
> argument?  Just to parallel the _DEFINE_LIB macros?  Seems unnecessary.

This was done to make it easier for the developers so they all had the same
calling format.  In my opinion, it would make them harder to use if there were
different parameter sequences and users had to know which macro had what format.

https://codereview.appspot.com/342870043/diff/60001/src/core/model/callback.cc
File src/core/model/callback.cc (right):

https://codereview.appspot.com/342870043/diff/60001/src/core/model/callback.c...
src/core/model/callback.cc:129: ns3::CallbackImplBase::Demangle (const
std::string& mangled)
On 2018/04/17 00:04:12, Peter Barnes wrote:
> Isn't this a bug in the current code?

Affirmative, for the Windows (old cygwin?) configuration only.  Since its a bug
in the Windows configuration only, I have addressed it in the Windows only
changes.

https://codereview.appspot.com/342870043/diff/60001/src/core/model/command-li...
File src/core/model/command-line.h (right):

https://codereview.appspot.com/342870043/diff/60001/src/core/model/command-li...
src/core/model/command-line.h:373: *  Function to force static initialization of
Time.
On 2018/04/17 00:04:12, Peter Barnes wrote:
> "CommandLine"

Corrected reference to Time.

https://codereview.appspot.com/342870043/diff/60001/src/core/model/fatal-impl.cc
File src/core/model/fatal-impl.cc (right):

https://codereview.appspot.com/342870043/diff/60001/src/core/model/fatal-impl...
src/core/model/fatal-impl.cc:210: std::list<std::ostream*> *l = *pl;
On 2018/04/17 00:04:12, Peter Barnes wrote:
> Seems like only 4-5 lines in this function are system dependent, so let's just
> put conditionals around them, so it's clear we're basically doing the same
> thing.
> 
> The conditional lines are the preceding three...

Updated.

https://codereview.appspot.com/342870043/diff/60001/src/core/model/fatal-impl...
src/core/model/fatal-impl.cc:221: signal(SIGSEGV, previousHandler);
On 2018/04/17 00:04:12, Peter Barnes wrote:
> ...and this one (which is two in the 'nix version)

Updated.

https://codereview.appspot.com/342870043/diff/60001/src/core/model/log-macros...
File src/core/model/log-macros-disabled.h (right):

https://codereview.appspot.com/342870043/diff/60001/src/core/model/log-macros...
src/core/model/log-macros-disabled.h:40: }                                      
\
On 2018/04/17 00:04:12, Peter Barnes wrote:
> Nope, can't delete the 'nix definition.  Also there's a new macro in this file
I
> just pushed.

Ok

https://codereview.appspot.com/342870043/diff/60001/src/core/model/log-macros...
File src/core/model/log-macros-enabled.h (right):

https://codereview.appspot.com/342870043/diff/60001/src/core/model/log-macros...
src/core/model/log-macros-enabled.h:86: __func__ << "(): ";                     
                 \
On 2018/04/17 00:04:12, Peter Barnes wrote:
> Discussed on the wiki:  can't we just use __func__ on both systems?

Unknown if that works on all platforms and I don't have a way to verify.

https://codereview.appspot.com/342870043/diff/60001/src/core/model/log-macros...
src/core/model/log-macros-enabled.h:214: << __func__ << "()" << std::endl;      
    \
On 2018/04/17 00:04:12, Peter Barnes wrote:
> Ditto

Ditto.

https://codereview.appspot.com/342870043/diff/60001/src/core/model/names.cc
File src/core/model/names.cc (right):

https://codereview.appspot.com/342870043/diff/60001/src/core/model/names.cc#n...
src/core/model/names.cc:684: return 0;
On 2018/04/17 00:04:12, Peter Barnes wrote:
> What's troublesome with this line?

Leaving this uncommented causes a code cannot be reached error.

https://codereview.appspot.com/342870043/diff/60001/src/core/model/object-base.h
File src/core/model/object-base.h (right):

https://codereview.appspot.com/342870043/diff/60001/src/core/model/object-bas...
src/core/model/object-base.h:88: #define
NS_OBJECT_TEMPLATE_CLASS_DEFINE(type,param)                    \
On 2018/04/17 00:04:13, Peter Barnes wrote:
> Can't this be implemented as
> 
> #define NS_OBJECT_TEMPLATE_CLASS_DEFINE(type,param)            \
>     NS_OBJECT_TEMPLATE_CLASS_DEFINE_LIB(type,param,)

No, lines 70 and 71 are different.

https://codereview.appspot.com/342870043/diff/60001/src/core/model/object-ptr...
File src/core/model/object-ptr-container.h (right):

https://codereview.appspot.com/342870043/diff/60001/src/core/model/object-ptr...
src/core/model/object-ptr-container.h:262: return
(obj->*m_get)(static_cast<uint32_t> (i));
On 2018/04/17 00:04:13, Peter Barnes wrote:
> I think this is a bug:  the three uses of std::size_t should be the template
> parameter INDEX

I had to do this to resolve a Visual Studio compiler warning in one of the
modules.  All examples and test function with this change.

https://codereview.appspot.com/342870043/diff/60001/src/core/model/system-pat...
File src/core/model/system-path.cc (right):

https://codereview.appspot.com/342870043/diff/60001/src/core/model/system-pat...
src/core/model/system-path.cc:339: #if defined(HAVE_MKDIR_H)
On 2018/04/17 00:04:13, Peter Barnes wrote:
> 1.  I've refactored this function to eliminate the extra create directory
below.
> 
> 2.  I've refactored this conditional section:
> 
>   bool makeDirErr;
> 
> #if ...
>   makeDirErr = ...
> #else
>   makeDirErr = ...
> #endif
> 
>   if (makeDirErr) ...

OK

https://codereview.appspot.com/342870043/diff/60001/src/core/model/system-thr...
File src/core/model/system-thread.cc (right):

https://codereview.appspot.com/342870043/diff/60001/src/core/model/system-thr...
src/core/model/system-thread.cc:99: NS_LOG_FUNCTION (id);
On 2018/04/17 00:04:13, Peter Barnes wrote:
> What's wrong with this line?

Error	C2679	binary '<<': no operator found which takes a right-hand operand of
type 'ns3::SystemThread::ThreadId' (or there is no acceptable conversion)

https://codereview.appspot.com/342870043/diff/60001/src/core/model/test.cc
File src/core/model/test.cc (right):

https://codereview.appspot.com/342870043/diff/60001/src/core/model/test.cc#ne...
src/core/model/test.cc:471: #else
On 2018/04/17 00:04:13, Peter Barnes wrote:
> Not clear what all this does.  Could you please elaborate what this does, why
> it's needed?  

Under Linux, NS_TEST_SOURCEDIR is defined by WAF.  Under Windows, the value of
NS_TEST_SOURCEDIR is defined by declaring it as NS_TEST_SOURCEDIR (__FILE__) in
the project settings file.

Using this approach, the value of NS_TEST_SOURCEDIR ends up being defined
slightly differently which the conditional code handles.

https://codereview.appspot.com/342870043/diff/60001/src/core/model/traced-val...
File src/core/model/traced-value.h (right):

https://codereview.appspot.com/342870043/diff/60001/src/core/model/traced-val...
src/core/model/traced-value.h:158: : m_v (other.Get ())
On 2018/04/17 00:04:13, Peter Barnes wrote:
> Instead, shouldn't this be a cast from U to T, as below?
> 
>   : m_v ( (T) (other.Get ()) )

That change just suppresses the warning.

https://codereview.appspot.com/342870043/diff/60001/src/core/model/uinteger.h
File src/core/model/uinteger.h (right):

https://codereview.appspot.com/342870043/diff/60001/src/core/model/uinteger.h...
src/core/model/uinteger.h:83: #include "ns3dll.h"
On 2018/04/17 00:04:13, Peter Barnes wrote:
> Doesnt' this have to be before the use of NS3CORELIB above?

Removed.

https://codereview.appspot.com/342870043/diff/60001/src/core/model/unix-fd-re...
File src/core/model/unix-fd-reader.cc (right):

https://codereview.appspot.com/342870043/diff/60001/src/core/model/unix-fd-re...
src/core/model/unix-fd-reader.cc:24: #ifndef _WIN32
On 2018/04/17 00:04:13, Peter Barnes wrote:
> We shouldn't have conditional compilation in this file; it's the unix
> implementation of FdREader.
> 
> We should create an generic header, fd-reader.h
> 
> We should select which to build (unix-fd-reader.cc or win32-rd-reader.cc) in
the
> wscript, not in conditional compilation.

changed to Win32 specific source files with a common header file

https://codereview.appspot.com/342870043/diff/60001/src/core/model/unix-syste...
File src/core/model/unix-system-condition.cc (right):

https://codereview.appspot.com/342870043/diff/60001/src/core/model/unix-syste...
src/core/model/unix-system-condition.cc:21: #ifndef _WIN32
On 2018/04/17 00:04:13, Peter Barnes wrote:
> See comment on unix-fd-reader.cc
> 
> And this should be in a separate header, suggest win32-time.h

Moved Windows declarations to a separate header file.

https://codereview.appspot.com/342870043/diff/60001/src/core/model/wall-clock...
File src/core/model/wall-clock-synchronizer.cc (right):

https://codereview.appspot.com/342870043/diff/60001/src/core/model/wall-clock...
src/core/model/wall-clock-synchronizer.cc:25: #ifndef _FILETIME_
On 2018/04/17 00:04:13, Peter Barnes wrote:
> This should probably be in a separate file, suggest win32-time.{h,cc}

Move to separate files.

https://codereview.appspot.com/342870043/diff/60001/src/core/model/wall-clock...
src/core/model/wall-clock-synchronizer.cc:408: #ifndef _WIN32
On 2018/04/17 00:04:13, Peter Barnes wrote:
> Why is this necessary?

Because it generates a never reachable code error (because its code that is
never reachable).

https://codereview.appspot.com/342870043/diff/60001/src/core/test/int64x64-te...
File src/core/test/int64x64-test-suite.cc (right):

https://codereview.appspot.com/342870043/diff/60001/src/core/test/int64x64-te...
src/core/test/int64x64-test-suite.cc:1007: #ifdef _MSC_VER
On 2018/04/17 00:04:13, Peter Barnes wrote:
> This should be in there previous if clause

Windows implementation is cairo_impl so its outside of the if clause.  epsilon
initialization move up to where other epsilon declaration is located.

https://codereview.appspot.com/342870043/diff/60001/src/core/test/threaded-te...
File src/core/test/threaded-test-suite.cc (right):

https://codereview.appspot.com/342870043/diff/60001/src/core/test/threaded-te...
src/core/test/threaded-test-suite.cc:110:
std::this_thread::sleep_for(std::chrono::nanoseconds(500));
On 2018/04/17 00:04:14, Peter Barnes wrote:
> Won't this work on Win?  It's standard C++-11.

incorporated your change.
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b