Skip to content

fix of incorrect type conversion.#562

Open
ihsinme wants to merge 1 commit intosctplab:masterfrom
ihsinme:patch-1
Open

fix of incorrect type conversion.#562
ihsinme wants to merge 1 commit intosctplab:masterfrom
ihsinme:patch-1

Conversation

@ihsinme
Copy link
Copy Markdown
Contributor

@ihsinme ihsinme commented Jan 4, 2021

used transform (uint64_t) (srtt * srtt) is incorrect. first, the product will be counted in uint32_t, and only then the conversion to uint64_t will be performed.

I couldn't find the importance of preemptive execution (srtt * srtt), so I suggest a simple fix.
however, if the sequence of the product in the expression ((uint64_t) net-> mtu * (uint64_t) (srtt * srtt)) is important, then I can correct the commit to any of the options:

  1. ((uint64_t) srtt * srtt * net-> mtu)
  2. ((uint64_t) net-> mtu * ((uint64_t) srtt * srtt))

I would also pay attention to the comparison if (srtt> 0), given the unsigned nature of srtt, I would suggest replacing it with if (srtt != 0).

used transform (uint64_t) (srtt * srtt) is incorrect. first, the product will be counted in uint32_t, and only then the conversion to uint64_t will be performed.

I couldn't find the importance of preemptive execution (srtt * srtt), so I suggest a simple fix.
however, if the sequence of the product in the expression ((uint64_t) net-> mtu * (uint64_t) (srtt * srtt)) is important, then I can correct the commit to any of the options:
1. ((uint64_t) srtt * srtt * net-> mtu)
2. ((uint64_t) net-> mtu * ((uint64_t) srtt * srtt))

I would also pay attention to the comparison if (srtt> 0), given the unsigned nature of srtt, I would suggest replacing it with if (srtt != 0).
@ihsinme
Copy link
Copy Markdown
Contributor Author

ihsinme commented Feb 5, 2021

good day.
tell me if I still need to do something in this PR.

@tuexen
Copy link
Copy Markdown
Member

tuexen commented Feb 5, 2021

No, it's fine. Just need to contemplate what the code should be...

@ihsinme
Copy link
Copy Markdown
Contributor Author

ihsinme commented Feb 11, 2022

is there any news on this PR?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants