Skip to content

Clarify borrowed ada_string lifetime and add owned host accessor#1091

Open
rootvector2 wants to merge 3 commits intoada-url:mainfrom
rootvector2:c-api-borrowed-string-lifetime
Open

Clarify borrowed ada_string lifetime and add owned host accessor#1091
rootvector2 wants to merge 3 commits intoada-url:mainfrom
rootvector2:c-api-borrowed-string-lifetime

Conversation

@rootvector2
Copy link
Copy Markdown

Fixes a lifetime issue in the C API where ada_string returned from getters is a borrowed view into internal storage and becomes invalid after mutation.

  • Documented lifetime behavior of ada_string in ada_c.h
  • Added ada_get_host_owned to provide a safe, owned copy
  • Added a small test to demonstrate correct usage

No breaking changes.

@anonrig anonrig requested a review from lemire March 20, 2026 12:33
include/ada_c.h Outdated

// Returns a heap-allocated copy of ada_get_host(result).
// Caller must free with ada_free_owned_string.
ada_owned_string ada_get_host_owned(ada_url result);
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.

Why do we need this? Should we also add ada_get_hostname_owned or all other variants? It doesn't make sense to me.

@rootvector2
Copy link
Copy Markdown
Author

I’ll keep this PR focused on clarifying the lifetime documentation and remove ada_get_host_owned to avoid partial API expansion.

If needed, I can propose a separate follow-up for a consistent owned-getter approach across all relevant getters.

include/ada_c.h Outdated
// string that is owned by the ada_url instance
//
// Lifetime rules:
// - This pointer is borrowed from the underlying ada_url object.
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.

It is not borrowed. It is a pointer to a string in the ada_url instance.

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.

Here is what grok says:

The phrase "a pointer is borrowed" almost always refers to the ownership and borrowing model in the Rust programming language.

include/ada_c.h Outdated
// url_aggregator getters
// if ada_is_valid(result)) is false, an empty string is returned
//
// Important: all ada_string results below are borrowed views into internal
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 don't think that this is correct. ada_get_origin Returns an ada_owned_string which you own (as the name implies). You can mutate the source ada_url all you want, it won't matter.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants