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.

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

bgpd, lib: Use frrstr_time() when using ctime_r() #17684

Merged
merged 4 commits into from
Dec 22, 2024

Conversation

ton31337
Copy link
Member

No description provided.

@ton31337 ton31337 force-pushed the fix/json_time_no_newline branch 5 times, most recently from 8c6fa66 to fc0bb7e Compare December 20, 2024 10:51
lib/frrstr.h Outdated Show resolved Hide resolved
newline is not expected to be printed in JSON outputs, e.g.:

```
"lastUpdate":{"epoch":1734490463,"string":"Wed Dec 18 04:54:23 2024\n"
```

Signed-off-by: Donatas Abraitis <[email protected]>
@ton31337 ton31337 force-pushed the fix/json_time_no_newline branch from fc0bb7e to 6dceb6f Compare December 20, 2024 15:27
Copy link
Contributor

@mjstapp mjstapp left a comment

Choose a reason for hiding this comment

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

It looks like there's some mixing here: a) there are some raw calls to ctime_r(), using some time value, and b) there's the existing lib api, that does some arithmetic and then calls ctime_r() on the result. and there are some places in the code where the code can be replaced with (b), but there are also some places where (a) is needed?

bgpd/bgp_btoa.c Outdated Show resolved Hide resolved
…-JSON

For JSON output we don't need newline to be printed.

Before:

```
"lastUpdate":{"epoch":1734490463,"string":"Wed Dec 18 04:54:23 2024\n"
```

After:

```
"lastUpdate":{"epoch":1734678560,"string":"Fri Dec 20 09:09:20 2024"
```

Signed-off-by: Donatas Abraitis <[email protected]>
Replace with time_to_string().

Signed-off-by: Donatas Abraitis <[email protected]>
@ton31337 ton31337 force-pushed the fix/json_time_no_newline branch from 6dceb6f to 8f5821e Compare December 20, 2024 15:59
@ton31337 ton31337 requested a review from mjstapp December 20, 2024 16:13
Copy link
Contributor

@mjstapp mjstapp 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 for cleaning that up

@Jafaral Jafaral merged commit e62c2f1 into FRRouting:master Dec 22, 2024
11 checks passed
@ton31337 ton31337 deleted the fix/json_time_no_newline branch December 22, 2024 07:44
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