Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Conversation

mzabaluev
Copy link
Contributor

Bring CString in line with other library functions on the failure mode conventions.
There should be no panic lurking.

Rendered

@mzabaluev
Copy link
Contributor Author

Cc @alexcrichton @aturon

@kennytm
Copy link
Member

kennytm commented Feb 14, 2015

Alternative: Add a checked_from_slice() method, just like we have x/y which may panic and x.checked_div(y) which does not.

@mzabaluev
Copy link
Contributor Author

@kennytm I have added a paragraph on why simply adding functions is worse than changing the "shortest name" API.

In case of division, a zero divisor arguably is a programmer error. A restriction on NULs in a string, on the other hand, is arbitrary for the value domain in general (a Unicode text with NUL characters is valid). There are also performance concerns regarding the implementation of division.

@mzabaluev
Copy link
Contributor Author

That said, a migration path is probably needed whereby the Result-returning functions are added under temporary names and the current functions deprecated. Suggestions?

@aturon
Copy link
Contributor

aturon commented Feb 17, 2015

Thanks so much for throwing this together! I'm increasingly in favor of this approach for C strings.

cc @wycats @nikomatsakis @brson @huonw

@pcwalton I know CString is somewhat common in Servo, do you all have a preference here?

@huonw
Copy link
Contributor

huonw commented Feb 17, 2015

Seems fine to me.

@pcwalton
Copy link
Contributor

@aturon +1 from me.

@aturon
Copy link
Contributor

aturon commented Feb 18, 2015

After discussion here and on discuss, we've decided to move forward with this long-requested change. Thanks again, @mzabaluev! We'll try to land this ahead of alpha2 if possible.

Tracking issue

@Centril Centril added A-panic Panics related proposals & ideas A-ffi FFI related proposals. labels Nov 23, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-ffi FFI related proposals. A-panic Panics related proposals & ideas

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants

Morty Proxy This is a proxified and sanitized view of the page, visit original site.