Bug 2590

Summary: Minor enhancements in red-queue-disc{.h, .cc}
Product: ns-3 Reporter: Mohit <tahiliani.nitk>
Component: traffic-controlAssignee: Stefano Avallone <stavallo>
Status: RESOLVED FIXED    
Severity: enhancement CC: ns-bugs, tomh
Priority: P3    
Version: ns-3-dev   
Hardware: All   
OS: All   
Attachments: Proposed changes
Proposed changes
Updated patch

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!