Repository navigation
Conversation
|
Other open pull requests touch the same code. Merging one will break the other, or both implement the same thing. If this one builds on them, list them in Depends on.
Some comparisons could not be completed because a pull request conflicts with the base branch. Review these shared files manually before concluding that there is no overlap.
|
9da4b11 to
08ea93a
Compare
|
The disassembly-backed findings are useful and the wording correctly distinguishes bundled-title evidence from console measurement. Exact-head |
|
Thanks for the review. I have converted this to a draft under the #5239 intake hold; there is no title run demonstrating a need for this documentation-only change. The #6713 overlap is an insertion beside the |
08ea93a to
8fcf18a
Compare
MarcZX10
left a comment
There was a problem hiding this comment.
I withdraw the earlier #5239 intake-hold objection. The two disassembly-backed debt updates clearly distinguish bundled libc.prx evidence from PS5 console measurements, and the qsort item has been split into #9688 as described. Exact-head whitespace and conventions pass against current main 3ca07d086d5f44fbe3c00370e7f03953ab46ca1b; the current-main merge tree is clean. Hosted conventions, eligibility and labels pass; product jobs are skipped. This documentation-only change needs no runtime test, and I did not claim a console run.
I refreshed the overlap check manually: merging #9613 with #9087 is clean, but a merge-tree comparison with #6713 produces a content conflict in docs/dev/debt/libc.md on the _Mtx_trylock entry. Please coordinate the landing order with #6713's author and rebase the second PR preserving both entries; update Depends on only if one actually must land first. The PR currently descends from 8b3a6ba88d8fc74e768f1a7e8e9e36fb69842768, while current main is 3ca07d086d5f44fbe3c00370e7f03953ab46ca1b; please update it to current main and refresh the overlap check. This request is for current branch and overlap coordination only, not the obsolete intake hold.
|
Correction to my preceding review: I ran the two pairwise merge-tree checks separately. Both #9087 and #6713 produce a content conflict in |
|
@MarcZX10 thanks for the correction. I merged all three locally on top of this head (8fcf18a): first #9087 (orezraey), then #6713 (karimibrahim07). Both conflicts are adjacency only, and neither PR changes the text of the two entries this PR edits:
Every added On landing order, this PR has no preference. Whichever of the three merges later only needs the resolution above. If one of #9087 or #6713 lands first, I'll rebase this onto it and keep their text unchanged. AI-assisted (Claude Code). |
|
Marked ready for review. This was a draft only because of the #5239 intake hold, which @MarcZX10 has withdrawn (#9613 (comment) and on #9596). |
|
@orezraey @karimibrahim07 could you confirm a landing order for the adjacent My proposed order is to let your implementation PRs land first and refresh this documentation-only PR afterward, preserving both of your debt updates. There is no code dependency on this PR, and I can also refresh after whichever one lands first if the other remains open. If this PR lands first, the previously documented adjacency resolution retains all three sets of changes. Current heads checked for this request: this PR |
|
@JJenkx agreed, any order works for #6713. Your resolution of the AI-assisted: yes |
|
Back to draft: @MarcZX10 clarified that the #5239 intake guidance is still active (#9688 (comment)). While the open queue is above 700, a PR should name a title shown to need the change on current |
8fcf18a to
f64e3bb
Compare
|
Rebased the single documentation commit onto current main The landing-order agreements with both #6713 and #9087 remain in the description. Pairwise checks still find additive conflicts only in |
|
I’ve noticed that asking you to hold this PR was unnecessary. It’s fine to keep submitting PRs; maintainers will handle them in due course. |
…rndup Both libc debt entries said the console behaviour was unverified. The libc.prx that PPSA13934 ships in sce_module/ settles them (read from disassembly, not measured on a console): - _Mtx_trylock (0x5cf0) passes its argument to scePthreadMutexTrylock and returns 0 on success, 3 for SCE_KERNEL_ERROR_EBUSY and 4 for any other error. - strdup (0x6b440) calls strlen and strndup (0x6b650) calls strnlen on the source with no null check, so both fault on a null source, except strndup(NULL, 0), which returns an empty copy where this libc returns null with EINVAL. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
f64e3bb to
684bbcc
Compare
|
Refreshed onto Repeated the pairwise merge-tree checks against #6713 |
What
Two libc debt entries said the console behaviour was unverified. This PR replaces that with what the
libc.prxPPSA13934 ships insce_module/does. I read it from disassembly, not by measuring on a console. Only the debt file changes._Mtx_trylock(k6pGNMwJB08, 0x5cf0, 33 bytes)upoVrzMHFeE#A#B(scePthreadMutexTrylock, through the PLT stub at 0xc43c0) and maps the result:strdup(g7zzzLDYGw0, 0x6b440) andstrndup(XGnuIBmEmyk, 0x6b650)strdupcallsstrlen(j4ViWNHEgww) on it.strndupcallsstrnlen(5jNubw4vlAA).strndup(NULL, 0): itsstrnlenreads nothing, and it returns an empty copy. Here that call returns null withEINVAL. The entry already explains why this libc returnsEINVALinstead of faulting, and now also records that one case.The
qsortreading that was a second commit here moved to #9688, which makesqsortandqsort_sfollow the bundled code and checks them against it.Tested
mainf57ceaad6bceeb00a7f4f09975f5932b2f20e28e; exact head684bbccaaed3cdad4abfd44d6da8a90b3ae8ceabhas one commit and no merge commits.python3 -I tools/check_conventions.py --base f57ceaad6bceeb00a7f4f09975f5932b2f20e28e --head HEADandgit diff --checkpass. Only the two existing debt entries change; no code or test changes and no runtime test claim.668d51adbdb91248f49fdd28acbfd0b85adb98cdand fix(libc): follow snprintf_s and sscanf_s constraints as measured on a PS5 #908749903de777725cb06d5a4f5abd57ddf286534650find conflicts only indocs/dev/debt/libc.md. The agreed retain-both resolutions below still apply; neither is a functional dependency.Landing order
The author of #6713 agrees that either landing order works. Resolve the additive
_Mtx_trylockdebt hunk by keeping main’s four entries, this PR’s_Mtx_trylockentry, then #6713’s “Application heap without process parameters” entry. If this PR lands first, #6713’s author will rebase accordingly. There is no code dependency between these PRs.The author of #9087 agrees to either landing order: #9087 can land first and this PR will be refreshed afterward. If this PR lands first, #9087’s author will rebase its
libc.mdhunk and keep these two entries unchanged. Preserve both PRs’ overlappingstrdup/strndupdebt edits. There is no code dependency between these PRs.Checklist
main; no other open PR implements the same functionsNotImplemented_nid_no_patch); silent stubs are listed under Silent stubs in their library's technical debt file.mdfiles or images added to the repository (attach them to this PR)🤖 Generated with Claude Code