Skip to content

Conversation

martinvuyk
Copy link
Contributor

@martinvuyk martinvuyk commented Apr 6, 2025

Replace _strnlen with _unsafe_strlen(ptr, max: UInt = UInt.MAX)

@martinvuyk martinvuyk requested a review from a team as a code owner April 6, 2025 20:48
@martinvuyk martinvuyk force-pushed the centralize-strlen-impls-and-make-safe branch 3 times, most recently from 7726c53 to ffe9887 Compare April 15, 2025 16:31
@martinvuyk martinvuyk force-pushed the centralize-strlen-impls-and-make-safe branch from 2062d66 to 9406f11 Compare April 15, 2025 20:00
@martinvuyk martinvuyk force-pushed the centralize-strlen-impls-and-make-safe branch 4 times, most recently from becf5ec to f46c599 Compare May 17, 2025 14:08
@soraros
Copy link
Contributor

soraros commented May 17, 2025

I suggest removing the _strlen variant and renaming _strnlen to _strlen. We don’t have to match the C API, and those functions are named differently only because C doesn’t support overloading or default arguments.

@martinvuyk martinvuyk force-pushed the centralize-strlen-impls-and-make-safe branch from f46c599 to 260acf2 Compare May 17, 2025 14:49
@martinvuyk martinvuyk changed the title [stdlib] [NFC] Move _strnlen to string_slice.mojo and make _unsafe_strlen a bit safer [stdlib] [NFC] Replace _strnlen with _unsafe_strlen(ptr, max: UInt = UInt.MAX) May 17, 2025
@martinvuyk martinvuyk force-pushed the centralize-strlen-impls-and-make-safe branch 2 times, most recently from b0f717e to 1de496f Compare May 17, 2025 15:01
@kartva kartva self-requested a review August 21, 2025 16:33
Copy link
Contributor

@kartva kartva left a comment

Choose a reason for hiding this comment

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

This PR is unifying two functions into one and generally makes the codebase better. The rename doesn't seem to be high-impact. LGTM!

You will need to rebase; there are a few small merge conflicts.

@martinvuyk martinvuyk force-pushed the centralize-strlen-impls-and-make-safe branch from 1de496f to e3425d7 Compare August 21, 2025 16:56
@kartva
Copy link
Contributor

kartva commented Aug 22, 2025

!sync

@modularbot modularbot added the imported-internally Signals that a given pull request has been imported internally. label Aug 22, 2025
@modularbot
Copy link
Collaborator

✅🟣 This contribution has been merged 🟣✅

Your pull request has been merged to the internal upstream Mojo sources. It will be reflected here in the Mojo repository on the main branch during the next Mojo nightly release, typically within the next 24-48 hours.

We use Copybara to merge external contributions, click here to learn more.

@modularbot modularbot added merged-internally Indicates that this pull request has been merged internally merged-externally Merged externally in public mojo repo labels Aug 22, 2025
@modularbot
Copy link
Collaborator

Landed in b1378c8! Thank you for your contribution 🎉

@github-actions github-actions bot locked and limited conversation to collaborators Aug 23, 2025
@martinvuyk martinvuyk deleted the centralize-strlen-impls-and-make-safe branch August 23, 2025 14:51
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
imported-internally Signals that a given pull request has been imported internally. merged-externally Merged externally in public mojo repo merged-internally Indicates that this pull request has been merged internally
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants