Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix ftime warning #3710

Merged
merged 3 commits into from
Sep 5, 2022
Merged

fix ftime warning #3710

merged 3 commits into from
Sep 5, 2022

Conversation

jiangying000
Copy link
Collaborator

Pull request type

Please check the type of change your PR introduces:

  • Bugfix
  • Feature
  • Code style update (formatting, renaming)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • Documentation content changes
  • Other (please describe):

What is the current behavior?

Issue Number: fix #3709

What is the new behavior?

Other information

@codecov
Copy link

codecov bot commented Sep 2, 2022 •
edited
Loading

Codecov Report

Merging #3710 (c091aa9) into master (740c36a) will increase coverage by 0.41%.
The diff coverage is 35.72%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #3710      +/-   ##
==========================================
+ Coverage   55.31%   55.71%   +0.41%     
==========================================
  Files         597      597              
  Lines       64331    64330       -1     
==========================================
+ Hits        35580    35838     +258     
+ Misses      28751    28492     -259
Flag Coverage Δ
unittests 55.71% <35.72%> (+0.41%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
abi/decoder/src/lib.rs 8.91% <0.00%> (+7.54%) ⬆️
account/service/src/service.rs 52.14% <0.00%> (ø)
chain/src/chain.rs 67.10% <ø> (-0.12%) ⬇️
chain/src/verifier/mod.rs 76.82% <ø> (ø)
cmd/starcoin/src/account/remove_cmd.rs 0.00% <ø> (ø)
commons/metrics/src/process_collector.rs 56.72% <0.00%> (ø)
commons/service-registry/src/service_ref.rs 62.76% <0.00%> (ø)
commons/stest/stest-macro/src/lib.rs 60.92% <ø> (ø)
miner/src/create_block_template/mod.rs 77.54% <ø> (ø)
sync/src/block_connector/write_block_chain.rs 85.19% <0.00%> (ø)
... and 30 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update dc3ec32...c091aa9. Read the comment docs.

nkysg
nkysg reviewed Sep 2, 2022
View reviewed changes
consensus/cryptonight-rs/ext/oaes_lib.c Outdated
@@ -518,6 +518,7 @@ static uint32_t oaes_get_seed(void)
gmTimer->tm_hour + gmTimer->tm_min + gmTimer->tm_sec + timer.millitm +
(uintptr_t) ( _test + timer.millitm ) + GETPID();
#else
#include <sys/time.h>
Copy link
Collaborator

Choose a reason for hiding this comment

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

https://man7.org/linux/man-pages/man3/ftime.3.html 参考这里可以使用time或gettimeofday, C programming include一般写在开头

Copy link
Collaborator Author

@jiangying000 jiangying000 Sep 3, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

把里面的 #include 去掉了，改成文件开始时判断条件 include，在glibc 2.31以上用gettimeofday，其他时候保持原样

@jiangying000 jiangying000 marked this pull request as ready for review September 3, 2022 04:43
jiangying000
jiangying000 commented Sep 3, 2022
View reviewed changes
Copy link
Collaborator Author

@jiangying000 jiangying000 left a comment

Choose a reason for hiding this comment

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

这些修改在 mac 和 window 不知道有没有问题

@jolestar
Copy link
Member

jolestar commented Sep 3, 2022

@sanlee42 看看

@jiangying000 jiangying000 requested a review from nkysg September 3, 2022 12:32
@nkysg
Copy link
Collaborator

nkysg commented Sep 5, 2022 •
edited
Loading

这些修改在 mac 和 window 不知道有没有问题

mac intel试了下没问题，看看哪位有windows可以帮忙试下

@jolestar jolestar merged commit b1d7e4d into starcoinorg:master Sep 5, 2022
@jolestar
Copy link
Member

好像是这个修改导致在 windows 下编译有问题

https://github.com/starcoinorg/starcoin/actions/runs/3132729622/jobs/5085372469

@jiangying000 jiangying000 mentioned this pull request Sep 27, 2022
Merged
7 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jolestar jolestar jolestar approved these changes

@sanlee42 sanlee42 Awaiting requested review from sanlee42 sanlee42 is a code owner

@LemonHX LemonHX Awaiting requested review from LemonHX

@nkysg nkysg Awaiting requested review from nkysg

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[Dev] building warning: warning: 'ftime' is deprecated [-Wdeprecated-declarations]...
3 participants
@jiangying000 @jolestar @nkysg