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

Issue 333080043: Eliminate Visual Studio complier warnings (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
11 months, 1 week ago by ammo6818-vandals.uidaho.edu
Modified:
7 months, 2 weeks ago
Reviewers:
Stefano Avallone
CC:
ns-3-reviews_googlegroups.com
Visibility:
Public.

Description

Eliminate Visual Studio complier warnings

Patch Set 1 #

Patch Set 2 : Updates to use C++ style type casts #

Total comments: 16

Patch Set 3 : Updated patch set to address review comments #

Total comments: 32

Patch Set 4 : Updates to patch for review comments #

Patch Set 5 : Patch updates for x64 build #

Patch Set 6 : Final change required to eliminate warnings #

Unified diffs Side-by-side diffs Delta from patch set Stats (+1 line, -1 line) Patch
M src/traffic-control/helper/traffic-control-helper.cc View 1 2 3 4 5 1 chunk +1 line, -1 line 0 comments Download

Messages

Total messages: 12
Stefano Avallone
https://codereview.appspot.com/333080043/diff/20001/src/traffic-control/examples/codel-vs-pfifo-asymmetric.cc File src/traffic-control/examples/codel-vs-pfifo-asymmetric.cc (right): https://codereview.appspot.com/333080043/diff/20001/src/traffic-control/examples/codel-vs-pfifo-asymmetric.cc#newcode235 src/traffic-control/examples/codel-vs-pfifo-asymmetric.cc:235: float startTime = 0.1f; OK. https://codereview.appspot.com/333080043/diff/20001/src/traffic-control/examples/codel-vs-pfifo-basic-test.cc File src/traffic-control/examples/codel-vs-pfifo-basic-test.cc (right): ...
9 months, 3 weeks ago (2017-12-28 23:55:28 UTC) #1
ammo6818-vandals.uidaho.edu
Updated patch set has been uploaded to address your review comments. https://codereview.appspot.com/333080043/diff/20001/src/traffic-control/examples/pfifo-vs-red.cc File src/traffic-control/examples/pfifo-vs-red.cc (right): ...
9 months, 3 weeks ago (2017-12-29 04:43:19 UTC) #2
Stefano Avallone
https://codereview.appspot.com/333080043/diff/60001/src/traffic-control/examples/adaptive-red-tests.cc File src/traffic-control/examples/adaptive-red-tests.cc (right): https://codereview.appspot.com/333080043/diff/60001/src/traffic-control/examples/adaptive-red-tests.cc#newcode180 src/traffic-control/examples/adaptive-red-tests.cc:180: #if 0 Why? https://codereview.appspot.com/333080043/diff/60001/src/traffic-control/examples/adaptive-red-tests.cc#newcode206 src/traffic-control/examples/adaptive-red-tests.cc:206: Why? https://codereview.appspot.com/333080043/diff/60001/src/traffic-control/examples/codel-vs-pfifo-basic-test.cc File src/traffic-control/examples/codel-vs-pfifo-basic-test.cc ...
9 months, 2 weeks ago (2017-12-29 22:56:39 UTC) #3
ammo6818-vandals.uidaho.edu
Updates for the latest review comments have been uploaded. https://codereview.appspot.com/333080043/diff/60001/src/traffic-control/examples/adaptive-red-tests.cc File src/traffic-control/examples/adaptive-red-tests.cc (right): https://codereview.appspot.com/333080043/diff/60001/src/traffic-control/examples/adaptive-red-tests.cc#newcode180 src/traffic-control/examples/adaptive-red-tests.cc:180: ...
9 months, 2 weeks ago (2017-12-31 05:15:04 UTC) #4
Tom Henderson
https://codereview.appspot.com/333080043/diff/60001/src/traffic-control/test/codel-queue-disc-test-suite.cc File src/traffic-control/test/codel-queue-disc-test-suite.cc (right): https://codereview.appspot.com/333080043/diff/60001/src/traffic-control/test/codel-queue-disc-test-suite.cc#newcode535 src/traffic-control/test/codel-queue-disc-test-suite.cc:535: NS_UNUSED(oldVal); On 2017/12/31 05:15:03, ammo6818-vandals.uidaho.edu wrote: > On 2017/12/29 ...
9 months, 2 weeks ago (2017-12-31 05:21:49 UTC) #5
ammo6818-vandals.uidaho.edu
Response provided to Tom's review comments about unused parameters. https://codereview.appspot.com/333080043/diff/60001/src/traffic-control/test/codel-queue-disc-test-suite.cc File src/traffic-control/test/codel-queue-disc-test-suite.cc (right): https://codereview.appspot.com/333080043/diff/60001/src/traffic-control/test/codel-queue-disc-test-suite.cc#newcode535 src/traffic-control/test/codel-queue-disc-test-suite.cc:535: ...
9 months, 2 weeks ago (2017-12-31 15:21:45 UTC) #6
Tom Henderson
> > yes, this warning can be suppressed in Visual Studio. however, before going to ...
9 months, 2 weeks ago (2017-12-31 18:57:21 UTC) #7
ammo6818-vandals.uidaho.edu
All review comments have been addressed and are ready for merging into ns-3-dev.
8 months ago (2018-02-11 19:43:38 UTC) #8
ammo6818-vandals.uidaho.edu
Please let me know if there are any other review comments or merge patches into ...
8 months ago (2018-02-18 05:42:34 UTC) #9
Stefano Avallone
Pushed to ns-3-dev with changeset 13307:111e0d0cb0ca. Thanks, Stefano
7 months, 3 weeks ago (2018-02-23 14:48:06 UTC) #10
ammo6818-vandals.uidaho.edu
There is one more change required to eliminate the final warning. Much thanks for your ...
7 months, 2 weeks ago (2018-02-28 20:33:43 UTC) #11
Stefano Avallone
7 months, 2 weeks ago (2018-03-01 10:46:14 UTC) #12
Yeah, I just pushed a fix that avoids casting and breaking build with clang.

Thanks.
Sign in to reply to this message.

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