align comments with code in decompiler #7
Conversation
Add a configuration option for the decompiler to align comments with code instead of using the fixed indentation setting.
|
Just tested merging master with this and it still works as expected. Screenshots attached. Current_Release is the behavior in 10.1.3.
Pull_Request is the behavior after running a merging master into this Pull Request and building it. |
|
Any E.T.A. on when/if this will be merged to release? |
|
Force-pushing a merge of this with the current |
|
Does your current patch work on top of master? I assume you also need changes to ElementIds: Hint: |
|
Thanks a ton for the heads-up on that, I didn't realize the mechanics of decompiler options were re-worked in such an extensive manner. It had indeed broken since the last check someone voluntarily performed. The new ordinal-style numbering of elements combined with the sequential manner in which they've been assigned makes it virtually impossible to create a new ElementId in a clean manner. The new option is now jammed lamely at the end, but there's no way to insert it with the other options without making the numbers jump in a non-nonsensical way. Shifting the ordinals of every other element seems even less desirable. I've chosen one of the two equally ugly options I see, and await feedback from the Ghidra team on preferred alternate courses of action. In the meantime, I tested this locally and it seems to work as expected, and the new tests pass once more. |
|
Warning Rate limit exceeded@visz11 has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 27 minutes and 18 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (8)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
|
/refacto-test |
|
Refacto is reviewing this PR. Please wait for the review comments to be posted. |
![refacto-test[bot]](https://avatars.githubusercontent.com/u/196302493?s=60&v=4){kind=small}
| indent = line_commentindent; // User specified default indent | ||
| emit->tagLine(indent); | ||
| if (line_commentindentalign) { | ||
| emit->tagLine(); |
There was a problem hiding this comment.
Security Issue: When comment alignment is enabled, there's no validation of indent values in the emitLineComment function. This could potentially lead to information disclosure by revealing unintended structural information about the code. Consider adding validation logic to handle edge cases when line_commentindentalign is true.
|
/refacto-test |
|
Refacto is reviewing this PR. Please wait for the review comments to be posted. |
|
/refacto-test |
|
Refacto is reviewing this PR. Please wait for the review comments to be posted. |
Code Review: Decompiler Comment Alignment👍 Well Done
📌 Files Processed
|
| mods = 0; | ||
| head_comment_type = Comment::header | Comment::warningheader; | ||
| line_commentindent = 20; | ||
| line_commentindentalign = false; |
There was a problem hiding this comment.
Missing Default Reset
The resetDefaultsInternal() method initializes line_commentindentalign but the variable isn't declared in the class. This creates a mismatch between initialization and declaration, potentially causing undefined behavior.
| mods = 0; | |
| head_comment_type = Comment::header | Comment::warningheader; | |
| line_commentindent = 20; | |
| line_commentindentalign = false; | |
| PrintLanguage::PrintLanguage(void) | |
| { | |
| emit = (EmitXml *)0; | |
| commenttype = 0; | |
| commentindent = -1; | |
| line_commentindentalign = false; | |
| namespaceStrategy = MINIMAL_NAMESPACES; | |
| } |
Standards
- ISO-IEC-25010-Reliability-Maturity
- ISO-IEC-25010-Functional-Correctness-Appropriateness
- DbC-Initialization
Add a configuration option for the decompiler to align comments with code instead of using the fixed indentation setting. See NationalSecurityAgency#575 for the original feature request.