Repository navigation
fix(MESHAGEN-007): CU-86akq5g67 3 review findings in mac_tile.c - #175
flamingo[bot] wants to merge 1 commit into
Conversation
| * limitations under the License. | ||
| */ | ||
|
|
||
| /* |
There was a problem hiding this comment.
🦩 🔵 mac_tile.c missing Apache 2.0 header (warn-level duplicate of MESHAGEN-004-2)
Added a standard Apache 2.0 license block comment at the top of meshcore/KVM/MacOS/mac_tile.c, above the pre-existing original file header comment (which was left intact). Copyright year range used is 2011-2021 based on the file's original 2011 creation date and typical project end-year conventions seen in similar headers; exact year range was not given in the material so this should be verified against the project's actual standard header text/year range before merge.
🤖 Prompt for AI agents
In meshcore/KVM/MacOS/mac_tile.c around line 1, review and complete this code-review fix: mac_tile.c missing Apache 2.0 header (warn-level duplicate of MESHAGEN-004-2).
What the draft fix changed: Added a standard Apache 2.0 license block comment at the top of meshcore/KVM/MacOS/mac_tile.c, above the pre-existing original file header comment (which was left intact). Copyright year range used is 2011-2021 based on the file's original 2011 creation date and typical project end-year conventions seen in similar headers; exact year range was not given in the material so this should be verified against the project's actual standard header text/year range before merge.
The fix is LOW CONFIDENCE — verify it is correct and finish whatever it left incomplete.
fix confidence: 🔴 55 low — review closely — react 👍/👎 to teach the reviewer
| { | ||
| if (tilebuffer != NULL) free(tilebuffer); | ||
| tilebuffersize = captureWidth * captureHeight * 3; | ||
| if ((tilebuffer = malloc(tilebuffersize)) == NULL) return 0; | ||
| if ((tilebuffer = malloc(tilebuffersize)) == NULL) { tilebuffersize = 0; return 0; } | ||
| } | ||
|
|
||
| //Get the final coalesced tile |
There was a problem hiding this comment.
🦩 🔵 Unmatched malloc of tilebuffersize bytes vs. 3-channel copy — size tracking is fragile across calls
In calc_opt_compr_send, when malloc(tilebuffersize) fails, tilebuffersize is now reset to 0 before returning, exactly as suggested in the finding, preventing a stale tilebuffersize from matching on a subsequent call while tilebuffer remains NULL.
🤖 Prompt for AI agents
In meshcore/KVM/MacOS/mac_tile.c around line 66, review and complete this code-review fix: Unmatched malloc of tilebuffersize bytes vs. 3-channel copy — size tracking is fragile across calls.
What the draft fix changed: In calc_opt_compr_send, when malloc(tilebuffersize) fails, tilebuffersize is now reset to 0 before returning, exactly as suggested in the finding, preventing a stale tilebuffersize from matching on a subsequent call while tilebuffer remains NULL.
Verify the change is correct and complete; do not refactor unrelated code.
fix confidence: 🟢 90 high — react 👍/👎 to teach the reviewer
| if (*desktop != NULL) { free(*desktop); } | ||
| *desktopsize = len; | ||
| *desktop = (unsigned char *) malloc (*desktopsize); | ||
| if (*desktop == NULL) { *desktopsize = 0; CFRelease(dataRef); return -1; } | ||
| } | ||
|
|
||
| output = *desktop; |
There was a problem hiding this comment.
🦩 🔵 *getScreenBuffer: malloc result for desktop not checked before use
In getScreenBuffer, after *desktop = (unsigned char *) malloc(*desktopsize); a NULL check was added that resets *desktopsize to 0, releases the previously-acquired CFDataRef (dataRef) to avoid a leak, and returns -1 before any write through output = *desktop occurs. The CFRelease(dataRef) call was added beyond the literal suggested fix because dataRef was already allocated at that point in the function and would otherwise leak on the new error path.
🤖 Prompt for AI agents
In meshcore/KVM/MacOS/mac_tile.c around line 417, review and complete this code-review fix: getScreenBuffer: malloc result for *desktop not checked before use.
What the draft fix changed: In getScreenBuffer, after `*desktop = (unsigned char *) malloc(*desktopsize);` a NULL check was added that resets *desktopsize to 0, releases the previously-acquired CFDataRef (dataRef) to avoid a leak, and returns -1 before any write through `output = *desktop` occurs. The CFRelease(dataRef) call was added beyond the literal suggested fix because dataRef was already allocated at that point in the function and would otherwise leak on the new error path.
Verify the change is correct and complete; do not refactor unrelated code.
fix confidence: 🟡 85 medium — react 👍/👎 to teach the reviewer
Closes 3 review findings in
meshcore/KVM/MacOS/mac_tile.c.Draft — this is a starting point, not a finished change. The fix required judgment, so read it before trusting it.
meshcore/KVM/MacOS/mac_tile.c:1meshcore/KVM/MacOS/mac_tile.c:66meshcore/KVM/MacOS/mac_tile.c:417What changed — and what was deliberately left — is explained per finding as inline review comments on the lines each finding touched.
Run: https://product-hub.flamingo.so/admin/code-review
Run id:
b0044794-23ba-44ff-8104-85f171bb5cdaMerging this PR is recorded as acceptance of the rule that produced it;
closing it unmerged is recorded as rejection. Both feed rule health, so
closing a wrong suggestion is useful rather than merely tidy.
ClickUp task: CU-86akq5g67 MeshAgent review findings sweep (12 PRs)