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

add some time functions on glibc and musl #2957

Merged
merged 1 commit into from
Oct 13, 2022
Merged

Conversation

SteveLauC
Copy link
Contributor

man pages

I didn't add ctime()/ctime_r() on musl because they involve the time_t type, which elicits a deprecation wraning.

Is it fine to add these two functions on musl, they will have the same definitions as the glibc ones:

pub fn ctime(timep: *const time_t) -> *mut ::c_char;
pub fn ctime_r(timep: *const time_t, buf: *mut ::c_char) -> *mut ::c_char;

If it's ok, I will add them:)

@rust-highfive
Copy link

r? @Amanieu

(rust-highfive has picked a reviewer for you, use r? to override)

@joshtriplett
Copy link
Member

While in general we provide bindings to most functions even if they're not advisable to call, is there a specific reason you need asctime (and ctime)? It returns a pointer to an internal libc buffer that gets overwritten, so it's very easy to misuse, and needs copying out for safety.

No objections if you really do need asctime, just want to confirm.

@SteveLauC
Copy link
Contributor Author

Yes, their static return value can be overwritten, which is not safe.

I don't have any specific reason to add them, since the reentrant functions provide the same functionality. I added them for the reason that not all platforms have such reentrant successors, so exposing them may keep some kind of consistency between those systems.

But they should be considered obsolete, will remove them:)

@JohnTitor
Copy link
Member

Thanks! @bors r+

@bors
Copy link
Contributor

bors commented Oct 12, 2022

📌 Commit bfc4064 has been approved by JohnTitor

It is now in the queue for this repository.

@bors
Copy link
Contributor

bors commented Oct 12, 2022

⌛ Testing commit bfc4064 with merge 2173b34...

bors added a commit that referenced this pull request Oct 12, 2022
add some time functions on glibc and musl

#### man pages

* [asctime/ctime man page](https://man7.org/linux/man-pages/man3/ctime.3.html)
* [strftime](https://man7.org/linux/man-pages/man3/strftime.3.html)
* [strptime](https://man7.org/linux/man-pages/man3/strptime.3.html)

I didn't add `ctime()/ctime_r()` on musl because they involve the `time_t` type, which elicits [a deprecation wraning](#1956).

Is it fine to add these two functions on `musl`, they will have the same definitions  as the `glibc` ones:
```rust
pub fn ctime(timep: *const time_t) -> *mut ::c_char;
pub fn ctime_r(timep: *const time_t, buf: *mut ::c_char) -> *mut ::c_char;
```
If it's ok, I will add them:)
@bors
Copy link
Contributor

bors commented Oct 12, 2022

💥 Test timed out

@JohnTitor
Copy link
Member

@bors retry

@bors
Copy link
Contributor

bors commented Oct 12, 2022

⌛ Testing commit bfc4064 with merge ed19cb9...

bors added a commit that referenced this pull request Oct 12, 2022
add some time functions on glibc and musl

#### man pages

* [asctime/ctime man page](https://man7.org/linux/man-pages/man3/ctime.3.html)
* [strftime](https://man7.org/linux/man-pages/man3/strftime.3.html)
* [strptime](https://man7.org/linux/man-pages/man3/strptime.3.html)

I didn't add `ctime()/ctime_r()` on musl because they involve the `time_t` type, which elicits [a deprecation wraning](#1956).

Is it fine to add these two functions on `musl`, they will have the same definitions  as the `glibc` ones:
```rust
pub fn ctime(timep: *const time_t) -> *mut ::c_char;
pub fn ctime_r(timep: *const time_t, buf: *mut ::c_char) -> *mut ::c_char;
```
If it's ok, I will add them:)
@bors
Copy link
Contributor

bors commented Oct 12, 2022

💥 Test timed out

@JohnTitor
Copy link
Member

@bors retry

@bors
Copy link
Contributor

bors commented Oct 13, 2022

⌛ Testing commit bfc4064 with merge 9960e1c...

@bors
Copy link
Contributor

bors commented Oct 13, 2022

☀️ Test successful - checks-actions, checks-cirrus-freebsd-12, checks-cirrus-freebsd-13, checks-cirrus-freebsd-14
Approved by: JohnTitor
Pushing 9960e1c to master...

1 similar comment
@bors
Copy link
Contributor

bors commented Oct 13, 2022

☀️ Test successful - checks-actions, checks-cirrus-freebsd-12, checks-cirrus-freebsd-13, checks-cirrus-freebsd-14
Approved by: JohnTitor
Pushing 9960e1c to master...

@bors bors merged commit 9960e1c into rust-lang:master Oct 13, 2022
@SteveLauC SteveLauC deleted the time-fn branch October 13, 2022 01:30
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.

6 participants