Skip to content

Refs #3468 Added methods to ease dds-security adaptation.#269

Merged
MiguelCompany merged 3 commits intodevelopfrom
feature/time_t_improvement
Oct 16, 2018
Merged

Refs #3468 Added methods to ease dds-security adaptation.#269
MiguelCompany merged 3 commits intodevelopfrom
feature/time_t_improvement

Conversation

@LuisGP
Copy link
Copy Markdown
Contributor

@LuisGP LuisGP commented Oct 8, 2018

Added some methods to easy Time_t (and Duration_t) class management:

  • Constructor from long double seconds.
  • to_ns method returns stored time as nanoseconds
  • >, >=, + and - operators.

Copy link
Copy Markdown
Member

@MiguelCompany MiguelCompany left a comment

Choose a reason for hiding this comment

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

Apart from the comments below, could you take the chance to change tabs into spaces on the whole file?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This will use 32 bit arithmetic. Please use ULL literals to all constants to force 64-bit arithmetic

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think this can produce a warning on some systems, as the result of the calculation is a long double. So I think a cast is required here.

@LuisGP
Copy link
Copy Markdown
Contributor Author

LuisGP commented Oct 8, 2018

Done, pong!

MiguelCompany
MiguelCompany previously approved these changes Oct 9, 2018
Copy link
Copy Markdown
Member

@MiguelCompany MiguelCompany left a comment

Choose a reason for hiding this comment

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

LGTM

@MiguelCompany MiguelCompany merged commit d52a4a9 into develop Oct 16, 2018
@MiguelCompany MiguelCompany deleted the feature/time_t_improvement branch October 16, 2018 10:39
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.

3 participants