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

Fix: voicevox_json_freeの対象が漏れていたことの修正 #571

Merged
merged 3 commits into from
Aug 10, 2023

Conversation

yamachu
Copy link
Member

@yamachu yamachu commented Aug 9, 2023

内容

C_STRING_DROP_CHECKER.whitelist に入れられた文字列は、Rust側でfreeする必要があるが、
user_dictが出力しているjson文字列も対象であることがdocumentから抜け落ちていた。

それに対応し、C-Headerを再生成した。

関連 Issue

その他

Copy link
Member

@qryxip qryxip left a comment

Choose a reason for hiding this comment

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

LGTM!

*const c_char (=const char*)を返すのって、Rustの言葉で言うと

  • static (rodata)
  • non-staticなborrowed (文字列の実体の「所有」者がデストラクトされると、ポインタが無効化されてしまう)
  • owned (voicevox_json_freeで解放)

の3つに分類されるので、説明の定型文をDoxygenの\コマンドにしてしまえば漏れがわかりやすくなるかなとちょっと思いました。

Copy link
Member

@Hiroshiba Hiroshiba left a comment

Choose a reason for hiding this comment

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

LGTM!!

@Hiroshiba Hiroshiba merged commit c22f1cf into VOICEVOX:main Aug 10, 2023
@yamachu yamachu deleted the json-free-docs branch September 6, 2023 04:49
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