Bug 2590 – Minor enhancements in red-queue-disc{.h, .cc}

Bug 2590 - Minor enhancements in red-queue-disc{.h, .cc}


Summary:	Minor enhancements in red-queue-disc{.h, .cc}

Status:	RESOLVED FIXED

Product:	ns-3
Classification:	Unclassified
Component:	traffic-control
Version:	ns-3-dev
Hardware:	All All

Importance:	P3 enhancement
Assigned To:	Stefano Avallone

URL:
Keywords:

Depends on:
Blocks:
	Show dependency tree / graph

Reported:	2016-12-16 10:55 UTC by Mohit
Modified:	2017-01-09 14:11 UTC (History)
CC List:	2 users (show)

See Also:

Attachments
Proposed changes (1.27 KB, patch) 2016-12-16 10:55 UTC, Mohit	Details \| Diff
Proposed changes (1.26 KB, patch) 2017-01-02 13:04 UTC, Mohit	Details \| Diff
Updated patch (2.04 KB, patch) 2017-01-02 13:56 UTC, Mohit	Details \| Diff
Show Obsolete (2) Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.

Description Mohit 2016-12-16 10:55:14 UTC

Created attachment 2705 [details]
Proposed changes

Following changes are proposed in the patch:

- Removed multiple includes of the same header file in red-queue-disc.h
- Corrected some typos in red-queue-disc.h
- Added a NS_LOG_FUNCTION which was missing in red-queue-disc.cc

Regards,
Mohit P. Tahiliani

Comment 1 Tom Henderson 2016-12-30 13:59:32 UTC

Regarding the log statement to UpdateMaxP():

 void
 RedQueueDisc::UpdateMaxP (double newAve, Time now)
 {
+  NS_LOG_FUNCTION (this << newAve << now);


I would like to remove the parameter 'Time now' since this is easily obtained from within the method.  I agree with the NS_LOG_FUNCTION that would then look like this:

 void
 RedQueueDisc::UpdateMaxP (double newAve)
 {
+  NS_LOG_FUNCTION (this << newAve);


I am fine with the other two corrections.

Comment 2 Mohit 2017-01-02 13:04:19 UTC

Created attachment 2724 [details]
Proposed changes

Thank you for the feedback.

I have uploaded a new patch with the suggested change.

Regards,
Mohit P. Tahiliani

Comment 3 Mohit 2017-01-02 13:10:48 UTC

Sorry, I did not notice earlier that you suggested to change the signature of the UpdateMaxP method itself, and not just the log statement.

I'll correct that too.

Comment 4 Mohit 2017-01-02 13:56:59 UTC

Created attachment 2725 [details]
Updated patch

I uploaded a new patch with the suggested changes in UpdateMaxP method.

Please let me know if this is fine.

Thanks,
Mohit P. Tahiliani

Comment 5 Stefano Avallone 2017-01-02 16:58:32 UTC

I will push these (and the other pending) changes next week as soon as I get back to my office PC (where I have configured the keys enabling me to push commits). At the moment I am not able to ssh into it unfortunately.

Thanks for the fixes!

Comment 6 Stefano Avallone 2017-01-09 14:11:56 UTC

Fixed by changeset 12521:7c23e2de62af, thanks!