Skip to content

Remove drand48() usage#8560

Merged
masaori335 merged 4 commits intoapache:masterfrom
v1siuol:issue_1907
Dec 24, 2021
Merged

Remove drand48() usage#8560
masaori335 merged 4 commits intoapache:masterfrom
v1siuol:issue_1907

Conversation

@v1siuol
Copy link
Copy Markdown
Contributor

@v1siuol v1siuol commented Dec 17, 2021

Fix #1907

@v1siuol v1siuol requested a review from bryancall as a code owner December 17, 2021 07:54
@v1siuol
Copy link
Copy Markdown
Contributor Author

v1siuol commented Dec 17, 2021

Minors in jtest:
We have duplicated #include <stdlib.h>, not sure if this is intentional.

MAX_FILE_ARGUMENTS is not used in this file any more. It was introduced in e8ad297. And the usage was removed in 25cd96d.

Please let me know if you have any questions. Thanks in advance.

@masaori335
Copy link
Copy Markdown
Contributor

Thanks for the cleanup! Looks reasonable.

We have duplicated #include <stdlib.h>, not sure if this is intentional.

MAX_FILE_ARGUMENTS is not used in this file any more. It was introduced in e8ad297. And the usage was removed in 25cd96d.

It looks like these are other cleanups we need to do. Let's do it on other PRs.

@bneradt please fire up CI jobs for this.

@bneradt
Copy link
Copy Markdown
Contributor

bneradt commented Dec 20, 2021

@v1siuol : Thank you for the PR. It looks like the clang-format build is failing. You can see the output from the run here:
https://ci.trafficserver.apache.org/job/Github_Builds/job/clang-format/476/console

This can most easily be addressed by running make clang-format in your local build tree. That will apply the fixes in place, allowing you to commit them and re-push your branch. The CI should re-run automatically then. @ mention me if it doesn't and I'll kick it off again.

masaori335
masaori335 previously approved these changes Dec 21, 2021
Copy link
Copy Markdown
Contributor

@masaori335 masaori335 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Look reasonable

@v1siuol
Copy link
Copy Markdown
Contributor Author

v1siuol commented Dec 21, 2021

Thanks @masaori335. I notice that there are a few more drand48() usages in the following files. Should I remove them as well?

trafficserver/example/plugins/c-api/thread_pool/test/SDKTest/psi_server.c
trafficserver/iocore/aio/test_AIO.cc
trafficserver/iocore/cache/CacheDir.cc
trafficserver/iocore/cache/CacheTest.cc
trafficserver/plugins/cache_promote/policy.cc
trafficserver/proxy/hdrs/unit_tests/test_Hdrs.cc
trafficserver/src/tscore/unit_tests/test_MMH.cc

@masaori335
Copy link
Copy Markdown
Contributor

Yes, ts::Random is introduced to replace it ( #7859 ). Either is fine to continue on this PR or make another PR.

Comment thread example/plugins/c-api/thread_pool/test/SDKTest/psi_server.c Outdated
Copy link
Copy Markdown
Contributor

@masaori335 masaori335 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please roll back the changes in the c file. Let's figure out the issue independently.

@v1siuol v1siuol changed the title Remove drand48() usage in jtest Remove drand48() usage Dec 22, 2021
Copy link
Copy Markdown
Contributor

@masaori335 masaori335 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. Thanks!

@masaori335 masaori335 merged commit 5d5920e into apache:master Dec 24, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Remove drand48() usage

3 participants