Skip to content

fixed MISRA rule violation. - #170

Open
parsley wants to merge 2 commits into
eclipse-threadx:devfrom
parsley:fix_misra
Open

fixed MISRA rule violation.#170
parsley wants to merge 2 commits into
eclipse-threadx:devfrom
parsley:fix_misra

Conversation

@parsley

Copy link
Copy Markdown
Contributor

This fixed a rule violation for my MISRA checker.

@fdesbiensfdesbiens left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for this, @parsley — you found a real defect, and you found all of it. The four macros you touched are every semicolon-terminated gx_* macro definition in common/inc/, and the eight whitespace fixes clear all remaining trailing whitespace in the file, so this won't come back as diff noise later.

I can't approve it as it stands, though, because of one out-of-diff casualty. Two things to sort out:

1. It breaks the GUIX Studio build.guix_studio/properties_win.cpp:2705 calls gx_system_dirty_mark(mpInfo->widget) with no semicolon of its own, leaning on the one the macro used to supply:

gx_system_dirty_mark(mpInfo->widget) // <-- no semicolon
mpProject->SetModified();

After your change that expands to _gx_system_dirty_mark((GX_WIDGET *)mpInfo->widget) mpProject->SetModified();, which is a hard syntax error in both the error-checking and non-error-checking configurations. Please add the missing ; there in this same PR.

That is the only such site — I swept the tree, and all 2315 gx_system_dirty_mark, 10 gx_progress_bar_info_set, and 36 gx_single_line_text_input_style_add references are otherwise either properly terminated or documentation comments.

Fair warning that our CI will not tell you this. All five workflows are gated on branches: [ master ] or workflow_dispatch, and this PR targets dev, so eclipsefdn/eca is the only check that runs. The breakage would have surfaced at the eventual devmaster merge and been blamed on the wrong change. That gap is ours to fix, not yours.

2. Please expand the description. "This fixed a rule violation for my MISRA checker" undersells the change considerably, and it doesn't name the rule — our convention is to cite it explicitly (this pattern is usually flagged under MISRA C:2004 Rule 19.4). More importantly, the correctness argument stands on its own without any checker; see my note on the first macro below.

One expectation to set for anyone reading this later: it does not make gx_api.h MISRA-clean. The macros still expand to function-call expressions rather than a parenthesised expression or a do { } while(0) construct, and the header's whole API-mapping design conflicts with MISRA C:2012 Dir 4.9. That is a deliberate, long-standing choice — this PR silences one diagnostic rather than resolving the category, which is fine and still worth doing.

Nothing else raises a flag: preprocessor-only, so identical generated code and no size or speed impact; no API/ABI change and so no rtos-docs-asciidoc update needed; the file already carries Copyright (c) 2026 Eclipse ThreadX contributors; and the branch targets dev correctly.

Requesting changes — happy to approve once properties_win.cpp:2705 has its semicolon.

Comment threadcommon/inc/gx_api.h
#define gx_single_line_text_input_position_get(a, b) _gx_single_line_text_input_position_get(a, b)
#define gx_single_line_text_input_right_arrow(a) _gx_single_line_text_input_right_arrow((GX_SINGLE_LINE_TEXT_INPUT *)a)
#define gx_single_line_text_input_style_add(a, b) _gx_single_line_text_input_style_add((GX_SINGLE_LINE_TEXT_INPUT *)a, b);
#define gx_single_line_text_input_style_add(a, b) _gx_single_line_text_input_style_add((GX_SINGLE_LINE_TEXT_INPUT *)a, b)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Worth stating in the PR description that this is a correctness fix, not just a checker complaint. Because these are UINT-returning services, the trailing semicolon made the documented API unusable in valid C:

if (gx_single_line_text_input_style_add(input, style) !=GX_SUCCESS) /* syntax error before this PR */if (cond) gx_single_line_text_input_style_add(input, style); else ... /* "else without a previous if" */

The first is exactly what a safety-conscious application does with a status-returning call. Note the _gxe_ counterpart at line 4794 was already correct, so this restores symmetry between the two blocks rather than changing a deliberate convention.

Comment threadcommon/inc/gx_api.h

#define gx_system_canvas_refresh _gx_system_canvas_refresh
#define gx_system_dirty_mark(a) _gx_system_dirty_mark((GX_WIDGET *)a);
#define gx_system_dirty_mark(a) _gx_system_dirty_mark((GX_WIDGET *)a)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This is the one that breaks the build.guix_studio/properties_win.cpp:2705 calls gx_system_dirty_mark(mpInfo->widget) without a trailing semicolon of its own and relies on the macro to supply it. Once this line changes, that expands into _gx_system_dirty_mark(...) mpProject->SetModified(); and fails to compile.

Please fix the call site in this PR — adding ; at properties_win.cpp:2705 is the whole change. It is the only site in the tree that depends on the embedded semicolon.

Comment threadcommon/inc/gx_api.h
#define gx_progress_bar_event_process _gxe_progress_bar_event_process
#define gx_progress_bar_font_set _gxe_progress_bar_font_set
#define gx_progress_bar_info_set(a, b) _gxe_progress_bar_info_set((GX_PROGRESS_BAR *)a, b);
#define gx_progress_bar_info_set(a, b) _gxe_progress_bar_info_set((GX_PROGRESS_BAR *)a, b)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Good catch on the asymmetry here: the _gx_ counterpart at line 3209 was already correct, so the defect existed only in the error-checking block — the mirror image of the gx_single_line_text_input_style_add case above, where only the non-error-checking block was affected. Easy pair to half-fix; you got both.

Comment threadcommon/inc/gx_api.h

#define gx_system_canvas_refresh _gxe_system_canvas_refresh
#define gx_system_dirty_mark(a) _gxe_system_dirty_mark((GX_WIDGET *)a);
#define gx_system_dirty_mark(a) _gxe_system_dirty_mark((GX_WIDGET *)a)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This one was wrong in both blocks, so with line 3364 the macro is now consistent across configurations.

Since nothing in the regression suite would have caught the original defect — test/guix_test/regression_test/tests/validation_guix_system_no_output.c:147 exercises _gx_system_dirty_mark directly and bypasses the macro entirely — could you add a compile-only check that uses each of the four macros in expression context? Something as small as:

if (gx_system_dirty_mark(widget) !=GX_SUCCESS)
{
/* ... */
}

It never has to run; failing to compile is the assertion. That is a one-time, near-zero-cost guard for this whole class of regression across the ~1000 API macros, and it satisfies the project's coverage requirement for the change.

Comment threadcommon/inc/gx_api.h
@@ -1099,7 +1099,7 @@ typedef struct GX_VIEW_STRUCT
GX_UBYTE gx_glyph_advance; /* Glyph advance */ \

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

No objection to bundling these with the semicolon fix — they're confined to the same file and leave it clean. For the record, I checked whether any of the eight sat after a line-continuation backslash, where trailing whitespace would be an actual defect rather than cosmetic: none did, including this one at the end of GX_GLYPH_MEMBERS_DECLARE. So these are purely cosmetic and carry no risk.

…eview <eclipse-threadx#170 (review)>).
- all other changes are trailing whitespaces removed automatically by VSCode.
@parsley

Copy link
Copy Markdown
ContributorAuthor

Thank you for the review of my PR.
Actually in my case I got a MISRA rule 14.3 violation due to the second semicolon I added in my own file where I used the marco.
I added the semicolon in properties_win.cpp and committed it.

Anything I missed?

@parsley
parsley requested a review from fdesbiensAugust 10, 2026 08:56
Sign up for freeto 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.

2 participants

@parsley@fdesbiens
, 'i'); if (__m === '*' || __re.test(location.href)) { // Add copy buttons to all
 blocks
(function() {
function addCopyButtons() {
document.querySelectorAll('pre code').forEach(function(codeBlock) {
if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;
codeBlock.parentElement.setAttribute('data-copy-added', 'true');
var btn = document.createElement('button');
btn.textContent = 'Copy';
btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';
btn.onmouseover = function() { this.style.opacity = '1'; };
btn.onmouseout = function() { this.style.opacity = '0.7'; };
btn.onclick = function() {
navigator.clipboard.writeText(codeBlock.textContent).then(function() {
btn.textContent = 'Copied!';
setTimeout(function() { btn.textContent = 'Copy'; }, 1500);
});
};
codeBlock.parentElement.style.position = 'relative';
codeBlock.parentElement.appendChild(btn);
});
}
addCopyButtons();
// Re-run on dynamic content
var observer = new MutationObserver(addCopyButtons);
observer.observe(document.body, { childList: true, subtree: true });
})();
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
fixed MISRA rule violation. by parsley · Pull Request #170 · eclipse-threadx/guix · GitHub
Skip to content

fixed MISRA rule violation. - #170

Open
parsley wants to merge 2 commits into
eclipse-threadx:devfrom
parsley:fix_misra
Open

fixed MISRA rule violation.#170
parsley wants to merge 2 commits into
eclipse-threadx:devfrom
parsley:fix_misra

Conversation

@parsley

Copy link
Copy Markdown
Contributor

This fixed a rule violation for my MISRA checker.

@fdesbiensfdesbiens left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for this, @parsley — you found a real defect, and you found all of it. The four macros you touched are every semicolon-terminated gx_* macro definition in common/inc/, and the eight whitespace fixes clear all remaining trailing whitespace in the file, so this won't come back as diff noise later.

I can't approve it as it stands, though, because of one out-of-diff casualty. Two things to sort out:

1. It breaks the GUIX Studio build.guix_studio/properties_win.cpp:2705 calls gx_system_dirty_mark(mpInfo->widget) with no semicolon of its own, leaning on the one the macro used to supply:

gx_system_dirty_mark(mpInfo->widget) // <-- no semicolon
mpProject->SetModified();

After your change that expands to _gx_system_dirty_mark((GX_WIDGET *)mpInfo->widget) mpProject->SetModified();, which is a hard syntax error in both the error-checking and non-error-checking configurations. Please add the missing ; there in this same PR.

That is the only such site — I swept the tree, and all 2315 gx_system_dirty_mark, 10 gx_progress_bar_info_set, and 36 gx_single_line_text_input_style_add references are otherwise either properly terminated or documentation comments.

Fair warning that our CI will not tell you this. All five workflows are gated on branches: [ master ] or workflow_dispatch, and this PR targets dev, so eclipsefdn/eca is the only check that runs. The breakage would have surfaced at the eventual devmaster merge and been blamed on the wrong change. That gap is ours to fix, not yours.

2. Please expand the description. "This fixed a rule violation for my MISRA checker" undersells the change considerably, and it doesn't name the rule — our convention is to cite it explicitly (this pattern is usually flagged under MISRA C:2004 Rule 19.4). More importantly, the correctness argument stands on its own without any checker; see my note on the first macro below.

One expectation to set for anyone reading this later: it does not make gx_api.h MISRA-clean. The macros still expand to function-call expressions rather than a parenthesised expression or a do { } while(0) construct, and the header's whole API-mapping design conflicts with MISRA C:2012 Dir 4.9. That is a deliberate, long-standing choice — this PR silences one diagnostic rather than resolving the category, which is fine and still worth doing.

Nothing else raises a flag: preprocessor-only, so identical generated code and no size or speed impact; no API/ABI change and so no rtos-docs-asciidoc update needed; the file already carries Copyright (c) 2026 Eclipse ThreadX contributors; and the branch targets dev correctly.

Requesting changes — happy to approve once properties_win.cpp:2705 has its semicolon.

Comment threadcommon/inc/gx_api.h
#define gx_single_line_text_input_position_get(a, b) _gx_single_line_text_input_position_get(a, b)
#define gx_single_line_text_input_right_arrow(a) _gx_single_line_text_input_right_arrow((GX_SINGLE_LINE_TEXT_INPUT *)a)
#define gx_single_line_text_input_style_add(a, b) _gx_single_line_text_input_style_add((GX_SINGLE_LINE_TEXT_INPUT *)a, b);
#define gx_single_line_text_input_style_add(a, b) _gx_single_line_text_input_style_add((GX_SINGLE_LINE_TEXT_INPUT *)a, b)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Worth stating in the PR description that this is a correctness fix, not just a checker complaint. Because these are UINT-returning services, the trailing semicolon made the documented API unusable in valid C:

if (gx_single_line_text_input_style_add(input, style) !=GX_SUCCESS) /* syntax error before this PR */if (cond) gx_single_line_text_input_style_add(input, style); else ... /* "else without a previous if" */

The first is exactly what a safety-conscious application does with a status-returning call. Note the _gxe_ counterpart at line 4794 was already correct, so this restores symmetry between the two blocks rather than changing a deliberate convention.

Comment threadcommon/inc/gx_api.h

#define gx_system_canvas_refresh _gx_system_canvas_refresh
#define gx_system_dirty_mark(a) _gx_system_dirty_mark((GX_WIDGET *)a);
#define gx_system_dirty_mark(a) _gx_system_dirty_mark((GX_WIDGET *)a)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This is the one that breaks the build.guix_studio/properties_win.cpp:2705 calls gx_system_dirty_mark(mpInfo->widget) without a trailing semicolon of its own and relies on the macro to supply it. Once this line changes, that expands into _gx_system_dirty_mark(...) mpProject->SetModified(); and fails to compile.

Please fix the call site in this PR — adding ; at properties_win.cpp:2705 is the whole change. It is the only site in the tree that depends on the embedded semicolon.

Comment threadcommon/inc/gx_api.h
#define gx_progress_bar_event_process _gxe_progress_bar_event_process
#define gx_progress_bar_font_set _gxe_progress_bar_font_set
#define gx_progress_bar_info_set(a, b) _gxe_progress_bar_info_set((GX_PROGRESS_BAR *)a, b);
#define gx_progress_bar_info_set(a, b) _gxe_progress_bar_info_set((GX_PROGRESS_BAR *)a, b)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Good catch on the asymmetry here: the _gx_ counterpart at line 3209 was already correct, so the defect existed only in the error-checking block — the mirror image of the gx_single_line_text_input_style_add case above, where only the non-error-checking block was affected. Easy pair to half-fix; you got both.

Comment threadcommon/inc/gx_api.h

#define gx_system_canvas_refresh _gxe_system_canvas_refresh
#define gx_system_dirty_mark(a) _gxe_system_dirty_mark((GX_WIDGET *)a);
#define gx_system_dirty_mark(a) _gxe_system_dirty_mark((GX_WIDGET *)a)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This one was wrong in both blocks, so with line 3364 the macro is now consistent across configurations.

Since nothing in the regression suite would have caught the original defect — test/guix_test/regression_test/tests/validation_guix_system_no_output.c:147 exercises _gx_system_dirty_mark directly and bypasses the macro entirely — could you add a compile-only check that uses each of the four macros in expression context? Something as small as:

if (gx_system_dirty_mark(widget) !=GX_SUCCESS)
{
/* ... */
}

It never has to run; failing to compile is the assertion. That is a one-time, near-zero-cost guard for this whole class of regression across the ~1000 API macros, and it satisfies the project's coverage requirement for the change.

Comment threadcommon/inc/gx_api.h
@@ -1099,7 +1099,7 @@ typedef struct GX_VIEW_STRUCT
GX_UBYTE gx_glyph_advance; /* Glyph advance */ \

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

No objection to bundling these with the semicolon fix — they're confined to the same file and leave it clean. For the record, I checked whether any of the eight sat after a line-continuation backslash, where trailing whitespace would be an actual defect rather than cosmetic: none did, including this one at the end of GX_GLYPH_MEMBERS_DECLARE. So these are purely cosmetic and carry no risk.

…eview <eclipse-threadx#170 (review)>).
- all other changes are trailing whitespaces removed automatically by VSCode.
@parsley

Copy link
Copy Markdown
ContributorAuthor

Thank you for the review of my PR.
Actually in my case I got a MISRA rule 14.3 violation due to the second semicolon I added in my own file where I used the marco.
I added the semicolon in properties_win.cpp and committed it.

Anything I missed?

@parsley
parsley requested a review from fdesbiensAugust 10, 2026 08:56
Sign up for freeto 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.

2 participants

@parsley@fdesbiens
, 'i'); if (__m === '*' || __re.test(location.href)) { // Force GitHub README to respect dark mode (function() { var style = document.createElement('style'); style.textContent = ' .markdown-body { color-scheme: dark light; } .markdown-body pre { background: #161b22 !important; } .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; } .markdown-body table th, .markdown-body table td { border-color: #30363d !important; } .markdown-body img { background: #0d1117; } .markdown-body blockquote { border-left-color: #8b949e; } .markdown-body hr { border-color: #30363d; } '; document.head.appendChild(style); })(); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ' fixed MISRA rule violation. by parsley · Pull Request #170 · eclipse-threadx/guix · GitHub
Skip to content

fixed MISRA rule violation. - #170

Open
parsley wants to merge 2 commits into
eclipse-threadx:devfrom
parsley:fix_misra
Open

fixed MISRA rule violation.#170
parsley wants to merge 2 commits into
eclipse-threadx:devfrom
parsley:fix_misra

Conversation

@parsley

Copy link
Copy Markdown
Contributor

This fixed a rule violation for my MISRA checker.

@fdesbiensfdesbiens left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for this, @parsley — you found a real defect, and you found all of it. The four macros you touched are every semicolon-terminated gx_* macro definition in common/inc/, and the eight whitespace fixes clear all remaining trailing whitespace in the file, so this won't come back as diff noise later.

I can't approve it as it stands, though, because of one out-of-diff casualty. Two things to sort out:

1. It breaks the GUIX Studio build.guix_studio/properties_win.cpp:2705 calls gx_system_dirty_mark(mpInfo->widget) with no semicolon of its own, leaning on the one the macro used to supply:

gx_system_dirty_mark(mpInfo->widget) // <-- no semicolon
mpProject->SetModified();

After your change that expands to _gx_system_dirty_mark((GX_WIDGET *)mpInfo->widget) mpProject->SetModified();, which is a hard syntax error in both the error-checking and non-error-checking configurations. Please add the missing ; there in this same PR.

That is the only such site — I swept the tree, and all 2315 gx_system_dirty_mark, 10 gx_progress_bar_info_set, and 36 gx_single_line_text_input_style_add references are otherwise either properly terminated or documentation comments.

Fair warning that our CI will not tell you this. All five workflows are gated on branches: [ master ] or workflow_dispatch, and this PR targets dev, so eclipsefdn/eca is the only check that runs. The breakage would have surfaced at the eventual devmaster merge and been blamed on the wrong change. That gap is ours to fix, not yours.

2. Please expand the description. "This fixed a rule violation for my MISRA checker" undersells the change considerably, and it doesn't name the rule — our convention is to cite it explicitly (this pattern is usually flagged under MISRA C:2004 Rule 19.4). More importantly, the correctness argument stands on its own without any checker; see my note on the first macro below.

One expectation to set for anyone reading this later: it does not make gx_api.h MISRA-clean. The macros still expand to function-call expressions rather than a parenthesised expression or a do { } while(0) construct, and the header's whole API-mapping design conflicts with MISRA C:2012 Dir 4.9. That is a deliberate, long-standing choice — this PR silences one diagnostic rather than resolving the category, which is fine and still worth doing.

Nothing else raises a flag: preprocessor-only, so identical generated code and no size or speed impact; no API/ABI change and so no rtos-docs-asciidoc update needed; the file already carries Copyright (c) 2026 Eclipse ThreadX contributors; and the branch targets dev correctly.

Requesting changes — happy to approve once properties_win.cpp:2705 has its semicolon.

Comment threadcommon/inc/gx_api.h
#define gx_single_line_text_input_position_get(a, b) _gx_single_line_text_input_position_get(a, b)
#define gx_single_line_text_input_right_arrow(a) _gx_single_line_text_input_right_arrow((GX_SINGLE_LINE_TEXT_INPUT *)a)
#define gx_single_line_text_input_style_add(a, b) _gx_single_line_text_input_style_add((GX_SINGLE_LINE_TEXT_INPUT *)a, b);
#define gx_single_line_text_input_style_add(a, b) _gx_single_line_text_input_style_add((GX_SINGLE_LINE_TEXT_INPUT *)a, b)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Worth stating in the PR description that this is a correctness fix, not just a checker complaint. Because these are UINT-returning services, the trailing semicolon made the documented API unusable in valid C:

if (gx_single_line_text_input_style_add(input, style) !=GX_SUCCESS) /* syntax error before this PR */if (cond) gx_single_line_text_input_style_add(input, style); else ... /* "else without a previous if" */

The first is exactly what a safety-conscious application does with a status-returning call. Note the _gxe_ counterpart at line 4794 was already correct, so this restores symmetry between the two blocks rather than changing a deliberate convention.

Comment threadcommon/inc/gx_api.h

#define gx_system_canvas_refresh _gx_system_canvas_refresh
#define gx_system_dirty_mark(a) _gx_system_dirty_mark((GX_WIDGET *)a);
#define gx_system_dirty_mark(a) _gx_system_dirty_mark((GX_WIDGET *)a)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This is the one that breaks the build.guix_studio/properties_win.cpp:2705 calls gx_system_dirty_mark(mpInfo->widget) without a trailing semicolon of its own and relies on the macro to supply it. Once this line changes, that expands into _gx_system_dirty_mark(...) mpProject->SetModified(); and fails to compile.

Please fix the call site in this PR — adding ; at properties_win.cpp:2705 is the whole change. It is the only site in the tree that depends on the embedded semicolon.

Comment threadcommon/inc/gx_api.h
#define gx_progress_bar_event_process _gxe_progress_bar_event_process
#define gx_progress_bar_font_set _gxe_progress_bar_font_set
#define gx_progress_bar_info_set(a, b) _gxe_progress_bar_info_set((GX_PROGRESS_BAR *)a, b);
#define gx_progress_bar_info_set(a, b) _gxe_progress_bar_info_set((GX_PROGRESS_BAR *)a, b)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Good catch on the asymmetry here: the _gx_ counterpart at line 3209 was already correct, so the defect existed only in the error-checking block — the mirror image of the gx_single_line_text_input_style_add case above, where only the non-error-checking block was affected. Easy pair to half-fix; you got both.

Comment threadcommon/inc/gx_api.h

#define gx_system_canvas_refresh _gxe_system_canvas_refresh
#define gx_system_dirty_mark(a) _gxe_system_dirty_mark((GX_WIDGET *)a);
#define gx_system_dirty_mark(a) _gxe_system_dirty_mark((GX_WIDGET *)a)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This one was wrong in both blocks, so with line 3364 the macro is now consistent across configurations.

Since nothing in the regression suite would have caught the original defect — test/guix_test/regression_test/tests/validation_guix_system_no_output.c:147 exercises _gx_system_dirty_mark directly and bypasses the macro entirely — could you add a compile-only check that uses each of the four macros in expression context? Something as small as:

if (gx_system_dirty_mark(widget) !=GX_SUCCESS)
{
/* ... */
}

It never has to run; failing to compile is the assertion. That is a one-time, near-zero-cost guard for this whole class of regression across the ~1000 API macros, and it satisfies the project's coverage requirement for the change.

Comment threadcommon/inc/gx_api.h
@@ -1099,7 +1099,7 @@ typedef struct GX_VIEW_STRUCT
GX_UBYTE gx_glyph_advance; /* Glyph advance */ \

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

No objection to bundling these with the semicolon fix — they're confined to the same file and leave it clean. For the record, I checked whether any of the eight sat after a line-continuation backslash, where trailing whitespace would be an actual defect rather than cosmetic: none did, including this one at the end of GX_GLYPH_MEMBERS_DECLARE. So these are purely cosmetic and carry no risk.

…eview <eclipse-threadx#170 (review)>).
- all other changes are trailing whitespaces removed automatically by VSCode.
@parsley

Copy link
Copy Markdown
ContributorAuthor

Thank you for the review of my PR.
Actually in my case I got a MISRA rule 14.3 violation due to the second semicolon I added in my own file where I used the marco.
I added the semicolon in properties_win.cpp and committed it.

Anything I missed?

@parsley
parsley requested a review from fdesbiensAugust 10, 2026 08:56
Sign up for freeto 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.

2 participants

@parsley@fdesbiens
, 'i'); if (__m === '*' || __re.test(location.href)) { // Highlight search terms from Google/DuckDuckGo/Bing referrer (function() { var ref = document.referrer; var terms = []; if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) { var url = new URL(ref); var q = url.searchParams.get('q') || url.searchParams.get('p'); if (q) { terms = q.split(/\s+/).filter(function(t) { return t.length > 2; }); } } if (terms.length === 0) return; var style = document.createElement('style'); style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }'; document.head.appendChild(style); function highlight(node) { if (node.nodeType === 3) { // text node var text = node.textContent; var found = false; terms.forEach(function(term) { var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\]\\]/g, '\\') + ')', 'gi'); if (regex.test(text)) { found = true; var frag = document.createDocumentFragment(); var parts = text.split(regex); parts.forEach(function(part, i) { if (i % 2 === 0) { frag.appendChild(document.createTextNode(part)); } else { var span = document.createElement('span'); span.className = 'userscript-highlight'; span.textContent = part; frag.appendChild(span); } }); node.parentNode.replaceChild(frag, node); } }); } else if (node.nodeType === 1 && node.childNodes) { // element var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT']; if (!skipTags.includes(node.tagName)) { Array.from(node.childNodes).forEach(highlight); } } } highlight(document.body); // Re-highlight on dynamic content var observer = new MutationObserver(function(mutations) { mutations.forEach(function(m) { m.addedNodes.forEach(function(node) { if (node.nodeType === 1 || node.nodeType === 3) highlight(node); }); }); }); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ' fixed MISRA rule violation. by parsley · Pull Request #170 · eclipse-threadx/guix · GitHub
Skip to content

fixed MISRA rule violation. - #170

Open
parsley wants to merge 2 commits into
eclipse-threadx:devfrom
parsley:fix_misra
Open

fixed MISRA rule violation.#170
parsley wants to merge 2 commits into
eclipse-threadx:devfrom
parsley:fix_misra

Conversation

@parsley

Copy link
Copy Markdown
Contributor

This fixed a rule violation for my MISRA checker.

@fdesbiensfdesbiens left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for this, @parsley — you found a real defect, and you found all of it. The four macros you touched are every semicolon-terminated gx_* macro definition in common/inc/, and the eight whitespace fixes clear all remaining trailing whitespace in the file, so this won't come back as diff noise later.

I can't approve it as it stands, though, because of one out-of-diff casualty. Two things to sort out:

1. It breaks the GUIX Studio build.guix_studio/properties_win.cpp:2705 calls gx_system_dirty_mark(mpInfo->widget) with no semicolon of its own, leaning on the one the macro used to supply:

gx_system_dirty_mark(mpInfo->widget) // <-- no semicolon
mpProject->SetModified();

After your change that expands to _gx_system_dirty_mark((GX_WIDGET *)mpInfo->widget) mpProject->SetModified();, which is a hard syntax error in both the error-checking and non-error-checking configurations. Please add the missing ; there in this same PR.

That is the only such site — I swept the tree, and all 2315 gx_system_dirty_mark, 10 gx_progress_bar_info_set, and 36 gx_single_line_text_input_style_add references are otherwise either properly terminated or documentation comments.

Fair warning that our CI will not tell you this. All five workflows are gated on branches: [ master ] or workflow_dispatch, and this PR targets dev, so eclipsefdn/eca is the only check that runs. The breakage would have surfaced at the eventual devmaster merge and been blamed on the wrong change. That gap is ours to fix, not yours.

2. Please expand the description. "This fixed a rule violation for my MISRA checker" undersells the change considerably, and it doesn't name the rule — our convention is to cite it explicitly (this pattern is usually flagged under MISRA C:2004 Rule 19.4). More importantly, the correctness argument stands on its own without any checker; see my note on the first macro below.

One expectation to set for anyone reading this later: it does not make gx_api.h MISRA-clean. The macros still expand to function-call expressions rather than a parenthesised expression or a do { } while(0) construct, and the header's whole API-mapping design conflicts with MISRA C:2012 Dir 4.9. That is a deliberate, long-standing choice — this PR silences one diagnostic rather than resolving the category, which is fine and still worth doing.

Nothing else raises a flag: preprocessor-only, so identical generated code and no size or speed impact; no API/ABI change and so no rtos-docs-asciidoc update needed; the file already carries Copyright (c) 2026 Eclipse ThreadX contributors; and the branch targets dev correctly.

Requesting changes — happy to approve once properties_win.cpp:2705 has its semicolon.

Comment threadcommon/inc/gx_api.h
#define gx_single_line_text_input_position_get(a, b) _gx_single_line_text_input_position_get(a, b)
#define gx_single_line_text_input_right_arrow(a) _gx_single_line_text_input_right_arrow((GX_SINGLE_LINE_TEXT_INPUT *)a)
#define gx_single_line_text_input_style_add(a, b) _gx_single_line_text_input_style_add((GX_SINGLE_LINE_TEXT_INPUT *)a, b);
#define gx_single_line_text_input_style_add(a, b) _gx_single_line_text_input_style_add((GX_SINGLE_LINE_TEXT_INPUT *)a, b)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Worth stating in the PR description that this is a correctness fix, not just a checker complaint. Because these are UINT-returning services, the trailing semicolon made the documented API unusable in valid C:

if (gx_single_line_text_input_style_add(input, style) !=GX_SUCCESS) /* syntax error before this PR */if (cond) gx_single_line_text_input_style_add(input, style); else ... /* "else without a previous if" */

The first is exactly what a safety-conscious application does with a status-returning call. Note the _gxe_ counterpart at line 4794 was already correct, so this restores symmetry between the two blocks rather than changing a deliberate convention.

Comment threadcommon/inc/gx_api.h

#define gx_system_canvas_refresh _gx_system_canvas_refresh
#define gx_system_dirty_mark(a) _gx_system_dirty_mark((GX_WIDGET *)a);
#define gx_system_dirty_mark(a) _gx_system_dirty_mark((GX_WIDGET *)a)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This is the one that breaks the build.guix_studio/properties_win.cpp:2705 calls gx_system_dirty_mark(mpInfo->widget) without a trailing semicolon of its own and relies on the macro to supply it. Once this line changes, that expands into _gx_system_dirty_mark(...) mpProject->SetModified(); and fails to compile.

Please fix the call site in this PR — adding ; at properties_win.cpp:2705 is the whole change. It is the only site in the tree that depends on the embedded semicolon.

Comment threadcommon/inc/gx_api.h
#define gx_progress_bar_event_process _gxe_progress_bar_event_process
#define gx_progress_bar_font_set _gxe_progress_bar_font_set
#define gx_progress_bar_info_set(a, b) _gxe_progress_bar_info_set((GX_PROGRESS_BAR *)a, b);
#define gx_progress_bar_info_set(a, b) _gxe_progress_bar_info_set((GX_PROGRESS_BAR *)a, b)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Good catch on the asymmetry here: the _gx_ counterpart at line 3209 was already correct, so the defect existed only in the error-checking block — the mirror image of the gx_single_line_text_input_style_add case above, where only the non-error-checking block was affected. Easy pair to half-fix; you got both.

Comment threadcommon/inc/gx_api.h

#define gx_system_canvas_refresh _gxe_system_canvas_refresh
#define gx_system_dirty_mark(a) _gxe_system_dirty_mark((GX_WIDGET *)a);
#define gx_system_dirty_mark(a) _gxe_system_dirty_mark((GX_WIDGET *)a)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This one was wrong in both blocks, so with line 3364 the macro is now consistent across configurations.

Since nothing in the regression suite would have caught the original defect — test/guix_test/regression_test/tests/validation_guix_system_no_output.c:147 exercises _gx_system_dirty_mark directly and bypasses the macro entirely — could you add a compile-only check that uses each of the four macros in expression context? Something as small as:

if (gx_system_dirty_mark(widget) !=GX_SUCCESS)
{
/* ... */
}

It never has to run; failing to compile is the assertion. That is a one-time, near-zero-cost guard for this whole class of regression across the ~1000 API macros, and it satisfies the project's coverage requirement for the change.

Comment threadcommon/inc/gx_api.h
@@ -1099,7 +1099,7 @@ typedef struct GX_VIEW_STRUCT
GX_UBYTE gx_glyph_advance; /* Glyph advance */ \

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

No objection to bundling these with the semicolon fix — they're confined to the same file and leave it clean. For the record, I checked whether any of the eight sat after a line-continuation backslash, where trailing whitespace would be an actual defect rather than cosmetic: none did, including this one at the end of GX_GLYPH_MEMBERS_DECLARE. So these are purely cosmetic and carry no risk.

…eview <eclipse-threadx#170 (review)>).
- all other changes are trailing whitespaces removed automatically by VSCode.
@parsley

Copy link
Copy Markdown
ContributorAuthor

Thank you for the review of my PR.
Actually in my case I got a MISRA rule 14.3 violation due to the second semicolon I added in my own file where I used the marco.
I added the semicolon in properties_win.cpp and committed it.

Anything I missed?

@parsley
parsley requested a review from fdesbiensAugust 10, 2026 08:56
Sign up for freeto 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.

2 participants

@parsley@fdesbiens
, 'i'); if (__m === '*' || __re.test(location.href)) { // Strip utm_, fbclid, gclid, etc. from all links on page (function() { var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content', 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid', 'ref', 'ref_src', 'source', 'medium', 'campaign']; function cleanUrl(url) { try { var u = new URL(url, window.location.origin); var changed = false; trackingParams.forEach(function(p) { if (u.searchParams.has(p)) { u.searchParams.delete(p); changed = true; } }); return changed ? u.toString() : url; } catch (e) { return url; } } function cleanLinks() { document.querySelectorAll('a[href]').forEach(function(a) { var clean = cleanUrl(a.href); if (clean !== a.href) a.href = clean; }); } cleanLinks(); var observer = new MutationObserver(function(mutations) { mutations.forEach(function(m) { m.addedNodes.forEach(function(node) { if (node.nodeType === 1) { if (node.tagName === 'A') cleanLinks(); node.querySelectorAll('a[href]').forEach(function(a) { var clean = cleanUrl(a.href); if (clean !== a.href) a.href = clean; }); } }); }); }); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + ' fixed MISRA rule violation. by parsley · Pull Request #170 · eclipse-threadx/guix · GitHub
Skip to content

fixed MISRA rule violation. - #170

Open
parsley wants to merge 2 commits into
eclipse-threadx:devfrom
parsley:fix_misra
Open

fixed MISRA rule violation.#170
parsley wants to merge 2 commits into
eclipse-threadx:devfrom
parsley:fix_misra

Conversation

@parsley

Copy link
Copy Markdown
Contributor

This fixed a rule violation for my MISRA checker.

@fdesbiensfdesbiens left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for this, @parsley — you found a real defect, and you found all of it. The four macros you touched are every semicolon-terminated gx_* macro definition in common/inc/, and the eight whitespace fixes clear all remaining trailing whitespace in the file, so this won't come back as diff noise later.

I can't approve it as it stands, though, because of one out-of-diff casualty. Two things to sort out:

1. It breaks the GUIX Studio build.guix_studio/properties_win.cpp:2705 calls gx_system_dirty_mark(mpInfo->widget) with no semicolon of its own, leaning on the one the macro used to supply:

gx_system_dirty_mark(mpInfo->widget) // <-- no semicolon
mpProject->SetModified();

After your change that expands to _gx_system_dirty_mark((GX_WIDGET *)mpInfo->widget) mpProject->SetModified();, which is a hard syntax error in both the error-checking and non-error-checking configurations. Please add the missing ; there in this same PR.

That is the only such site — I swept the tree, and all 2315 gx_system_dirty_mark, 10 gx_progress_bar_info_set, and 36 gx_single_line_text_input_style_add references are otherwise either properly terminated or documentation comments.

Fair warning that our CI will not tell you this. All five workflows are gated on branches: [ master ] or workflow_dispatch, and this PR targets dev, so eclipsefdn/eca is the only check that runs. The breakage would have surfaced at the eventual devmaster merge and been blamed on the wrong change. That gap is ours to fix, not yours.

2. Please expand the description. "This fixed a rule violation for my MISRA checker" undersells the change considerably, and it doesn't name the rule — our convention is to cite it explicitly (this pattern is usually flagged under MISRA C:2004 Rule 19.4). More importantly, the correctness argument stands on its own without any checker; see my note on the first macro below.

One expectation to set for anyone reading this later: it does not make gx_api.h MISRA-clean. The macros still expand to function-call expressions rather than a parenthesised expression or a do { } while(0) construct, and the header's whole API-mapping design conflicts with MISRA C:2012 Dir 4.9. That is a deliberate, long-standing choice — this PR silences one diagnostic rather than resolving the category, which is fine and still worth doing.

Nothing else raises a flag: preprocessor-only, so identical generated code and no size or speed impact; no API/ABI change and so no rtos-docs-asciidoc update needed; the file already carries Copyright (c) 2026 Eclipse ThreadX contributors; and the branch targets dev correctly.

Requesting changes — happy to approve once properties_win.cpp:2705 has its semicolon.

Comment threadcommon/inc/gx_api.h
#define gx_single_line_text_input_position_get(a, b) _gx_single_line_text_input_position_get(a, b)
#define gx_single_line_text_input_right_arrow(a) _gx_single_line_text_input_right_arrow((GX_SINGLE_LINE_TEXT_INPUT *)a)
#define gx_single_line_text_input_style_add(a, b) _gx_single_line_text_input_style_add((GX_SINGLE_LINE_TEXT_INPUT *)a, b);
#define gx_single_line_text_input_style_add(a, b) _gx_single_line_text_input_style_add((GX_SINGLE_LINE_TEXT_INPUT *)a, b)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Worth stating in the PR description that this is a correctness fix, not just a checker complaint. Because these are UINT-returning services, the trailing semicolon made the documented API unusable in valid C:

if (gx_single_line_text_input_style_add(input, style) !=GX_SUCCESS) /* syntax error before this PR */if (cond) gx_single_line_text_input_style_add(input, style); else ... /* "else without a previous if" */

The first is exactly what a safety-conscious application does with a status-returning call. Note the _gxe_ counterpart at line 4794 was already correct, so this restores symmetry between the two blocks rather than changing a deliberate convention.

Comment threadcommon/inc/gx_api.h

#define gx_system_canvas_refresh _gx_system_canvas_refresh
#define gx_system_dirty_mark(a) _gx_system_dirty_mark((GX_WIDGET *)a);
#define gx_system_dirty_mark(a) _gx_system_dirty_mark((GX_WIDGET *)a)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This is the one that breaks the build.guix_studio/properties_win.cpp:2705 calls gx_system_dirty_mark(mpInfo->widget) without a trailing semicolon of its own and relies on the macro to supply it. Once this line changes, that expands into _gx_system_dirty_mark(...) mpProject->SetModified(); and fails to compile.

Please fix the call site in this PR — adding ; at properties_win.cpp:2705 is the whole change. It is the only site in the tree that depends on the embedded semicolon.

Comment threadcommon/inc/gx_api.h
#define gx_progress_bar_event_process _gxe_progress_bar_event_process
#define gx_progress_bar_font_set _gxe_progress_bar_font_set
#define gx_progress_bar_info_set(a, b) _gxe_progress_bar_info_set((GX_PROGRESS_BAR *)a, b);
#define gx_progress_bar_info_set(a, b) _gxe_progress_bar_info_set((GX_PROGRESS_BAR *)a, b)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Good catch on the asymmetry here: the _gx_ counterpart at line 3209 was already correct, so the defect existed only in the error-checking block — the mirror image of the gx_single_line_text_input_style_add case above, where only the non-error-checking block was affected. Easy pair to half-fix; you got both.

Comment threadcommon/inc/gx_api.h

#define gx_system_canvas_refresh _gxe_system_canvas_refresh
#define gx_system_dirty_mark(a) _gxe_system_dirty_mark((GX_WIDGET *)a);
#define gx_system_dirty_mark(a) _gxe_system_dirty_mark((GX_WIDGET *)a)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This one was wrong in both blocks, so with line 3364 the macro is now consistent across configurations.

Since nothing in the regression suite would have caught the original defect — test/guix_test/regression_test/tests/validation_guix_system_no_output.c:147 exercises _gx_system_dirty_mark directly and bypasses the macro entirely — could you add a compile-only check that uses each of the four macros in expression context? Something as small as:

if (gx_system_dirty_mark(widget) !=GX_SUCCESS)
{
/* ... */
}

It never has to run; failing to compile is the assertion. That is a one-time, near-zero-cost guard for this whole class of regression across the ~1000 API macros, and it satisfies the project's coverage requirement for the change.

Comment threadcommon/inc/gx_api.h
@@ -1099,7 +1099,7 @@ typedef struct GX_VIEW_STRUCT
GX_UBYTE gx_glyph_advance; /* Glyph advance */ \

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

No objection to bundling these with the semicolon fix — they're confined to the same file and leave it clean. For the record, I checked whether any of the eight sat after a line-continuation backslash, where trailing whitespace would be an actual defect rather than cosmetic: none did, including this one at the end of GX_GLYPH_MEMBERS_DECLARE. So these are purely cosmetic and carry no risk.

…eview <eclipse-threadx#170 (review)>).
- all other changes are trailing whitespaces removed automatically by VSCode.
@parsley

Copy link
Copy Markdown
ContributorAuthor

Thank you for the review of my PR.
Actually in my case I got a MISRA rule 14.3 violation due to the second semicolon I added in my own file where I used the marco.
I added the semicolon in properties_win.cpp and committed it.

Anything I missed?

@parsley
parsley requested a review from fdesbiensAugust 10, 2026 08:56
Sign up for freeto 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.

2 participants

@parsley@fdesbiens
, 'i'); if (__m === '*' || __re.test(location.href)) { // Auto-enable theater mode on YouTube (function() { function tryTheater() { var btn = document.querySelector('button[aria-label="Theater mode"], ytd-player #player button[title="Theater mode"]'); if (btn && !btn.classList.contains('activated')) { btn.click(); } } // Try immediately tryTheater(); // Try after navigation (SPA) var lastUrl = location.href; setInterval(function() { if (location.href !== lastUrl) { lastUrl = location.href; setTimeout(tryTheater, 500); } }, 1000); // Also try on player load var observer = new MutationObserver(tryTheater); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ' fixed MISRA rule violation. by parsley · Pull Request #170 · eclipse-threadx/guix · GitHub
Skip to content

fixed MISRA rule violation. - #170

Open
parsley wants to merge 2 commits into
eclipse-threadx:devfrom
parsley:fix_misra
Open

fixed MISRA rule violation.#170
parsley wants to merge 2 commits into
eclipse-threadx:devfrom
parsley:fix_misra

Conversation

@parsley

Copy link
Copy Markdown
Contributor

This fixed a rule violation for my MISRA checker.

@fdesbiensfdesbiens left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for this, @parsley — you found a real defect, and you found all of it. The four macros you touched are every semicolon-terminated gx_* macro definition in common/inc/, and the eight whitespace fixes clear all remaining trailing whitespace in the file, so this won't come back as diff noise later.

I can't approve it as it stands, though, because of one out-of-diff casualty. Two things to sort out:

1. It breaks the GUIX Studio build.guix_studio/properties_win.cpp:2705 calls gx_system_dirty_mark(mpInfo->widget) with no semicolon of its own, leaning on the one the macro used to supply:

gx_system_dirty_mark(mpInfo->widget) // <-- no semicolon
mpProject->SetModified();

After your change that expands to _gx_system_dirty_mark((GX_WIDGET *)mpInfo->widget) mpProject->SetModified();, which is a hard syntax error in both the error-checking and non-error-checking configurations. Please add the missing ; there in this same PR.

That is the only such site — I swept the tree, and all 2315 gx_system_dirty_mark, 10 gx_progress_bar_info_set, and 36 gx_single_line_text_input_style_add references are otherwise either properly terminated or documentation comments.

Fair warning that our CI will not tell you this. All five workflows are gated on branches: [ master ] or workflow_dispatch, and this PR targets dev, so eclipsefdn/eca is the only check that runs. The breakage would have surfaced at the eventual devmaster merge and been blamed on the wrong change. That gap is ours to fix, not yours.

2. Please expand the description. "This fixed a rule violation for my MISRA checker" undersells the change considerably, and it doesn't name the rule — our convention is to cite it explicitly (this pattern is usually flagged under MISRA C:2004 Rule 19.4). More importantly, the correctness argument stands on its own without any checker; see my note on the first macro below.

One expectation to set for anyone reading this later: it does not make gx_api.h MISRA-clean. The macros still expand to function-call expressions rather than a parenthesised expression or a do { } while(0) construct, and the header's whole API-mapping design conflicts with MISRA C:2012 Dir 4.9. That is a deliberate, long-standing choice — this PR silences one diagnostic rather than resolving the category, which is fine and still worth doing.

Nothing else raises a flag: preprocessor-only, so identical generated code and no size or speed impact; no API/ABI change and so no rtos-docs-asciidoc update needed; the file already carries Copyright (c) 2026 Eclipse ThreadX contributors; and the branch targets dev correctly.

Requesting changes — happy to approve once properties_win.cpp:2705 has its semicolon.

Comment threadcommon/inc/gx_api.h
#define gx_single_line_text_input_position_get(a, b) _gx_single_line_text_input_position_get(a, b)
#define gx_single_line_text_input_right_arrow(a) _gx_single_line_text_input_right_arrow((GX_SINGLE_LINE_TEXT_INPUT *)a)
#define gx_single_line_text_input_style_add(a, b) _gx_single_line_text_input_style_add((GX_SINGLE_LINE_TEXT_INPUT *)a, b);
#define gx_single_line_text_input_style_add(a, b) _gx_single_line_text_input_style_add((GX_SINGLE_LINE_TEXT_INPUT *)a, b)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Worth stating in the PR description that this is a correctness fix, not just a checker complaint. Because these are UINT-returning services, the trailing semicolon made the documented API unusable in valid C:

if (gx_single_line_text_input_style_add(input, style) !=GX_SUCCESS) /* syntax error before this PR */if (cond) gx_single_line_text_input_style_add(input, style); else ... /* "else without a previous if" */

The first is exactly what a safety-conscious application does with a status-returning call. Note the _gxe_ counterpart at line 4794 was already correct, so this restores symmetry between the two blocks rather than changing a deliberate convention.

Comment threadcommon/inc/gx_api.h

#define gx_system_canvas_refresh _gx_system_canvas_refresh
#define gx_system_dirty_mark(a) _gx_system_dirty_mark((GX_WIDGET *)a);
#define gx_system_dirty_mark(a) _gx_system_dirty_mark((GX_WIDGET *)a)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This is the one that breaks the build.guix_studio/properties_win.cpp:2705 calls gx_system_dirty_mark(mpInfo->widget) without a trailing semicolon of its own and relies on the macro to supply it. Once this line changes, that expands into _gx_system_dirty_mark(...) mpProject->SetModified(); and fails to compile.

Please fix the call site in this PR — adding ; at properties_win.cpp:2705 is the whole change. It is the only site in the tree that depends on the embedded semicolon.

Comment threadcommon/inc/gx_api.h
#define gx_progress_bar_event_process _gxe_progress_bar_event_process
#define gx_progress_bar_font_set _gxe_progress_bar_font_set
#define gx_progress_bar_info_set(a, b) _gxe_progress_bar_info_set((GX_PROGRESS_BAR *)a, b);
#define gx_progress_bar_info_set(a, b) _gxe_progress_bar_info_set((GX_PROGRESS_BAR *)a, b)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Good catch on the asymmetry here: the _gx_ counterpart at line 3209 was already correct, so the defect existed only in the error-checking block — the mirror image of the gx_single_line_text_input_style_add case above, where only the non-error-checking block was affected. Easy pair to half-fix; you got both.

Comment threadcommon/inc/gx_api.h

#define gx_system_canvas_refresh _gxe_system_canvas_refresh
#define gx_system_dirty_mark(a) _gxe_system_dirty_mark((GX_WIDGET *)a);
#define gx_system_dirty_mark(a) _gxe_system_dirty_mark((GX_WIDGET *)a)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This one was wrong in both blocks, so with line 3364 the macro is now consistent across configurations.

Since nothing in the regression suite would have caught the original defect — test/guix_test/regression_test/tests/validation_guix_system_no_output.c:147 exercises _gx_system_dirty_mark directly and bypasses the macro entirely — could you add a compile-only check that uses each of the four macros in expression context? Something as small as:

if (gx_system_dirty_mark(widget) !=GX_SUCCESS)
{
/* ... */
}

It never has to run; failing to compile is the assertion. That is a one-time, near-zero-cost guard for this whole class of regression across the ~1000 API macros, and it satisfies the project's coverage requirement for the change.

Comment threadcommon/inc/gx_api.h
@@ -1099,7 +1099,7 @@ typedef struct GX_VIEW_STRUCT
GX_UBYTE gx_glyph_advance; /* Glyph advance */ \

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

No objection to bundling these with the semicolon fix — they're confined to the same file and leave it clean. For the record, I checked whether any of the eight sat after a line-continuation backslash, where trailing whitespace would be an actual defect rather than cosmetic: none did, including this one at the end of GX_GLYPH_MEMBERS_DECLARE. So these are purely cosmetic and carry no risk.

…eview <eclipse-threadx#170 (review)>).
- all other changes are trailing whitespaces removed automatically by VSCode.
@parsley

Copy link
Copy Markdown
ContributorAuthor

Thank you for the review of my PR.
Actually in my case I got a MISRA rule 14.3 violation due to the second semicolon I added in my own file where I used the marco.
I added the semicolon in properties_win.cpp and committed it.

Anything I missed?

@parsley
parsley requested a review from fdesbiensAugust 10, 2026 08:56
Sign up for freeto 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.

2 participants

@parsley@fdesbiens
, 'i'); if (__m === '*' || __re.test(location.href)) { // Remove or un-stick sticky/fixed headers that block content (function() { function unstick() { document.querySelectorAll('header, nav, [role="banner"], .header, .navbar, .sticky, .fixed-top, [style*="position: fixed"], [style*="position:sticky"]').forEach(function(el) { if (el.style.position === 'fixed' || el.style.position === 'sticky' || getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') { el.style.position = 'static'; el.style.top = 'auto'; el.style.zIndex = 'auto'; } }); } unstick(); var observer = new MutationObserver(unstick); observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] }); })(); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ' fixed MISRA rule violation. by parsley · Pull Request #170 · eclipse-threadx/guix · GitHub
Skip to content

fixed MISRA rule violation. - #170

Open
parsley wants to merge 2 commits into
eclipse-threadx:devfrom
parsley:fix_misra
Open

fixed MISRA rule violation.#170
parsley wants to merge 2 commits into
eclipse-threadx:devfrom
parsley:fix_misra

Conversation

@parsley

Copy link
Copy Markdown
Contributor

This fixed a rule violation for my MISRA checker.

@fdesbiensfdesbiens left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for this, @parsley — you found a real defect, and you found all of it. The four macros you touched are every semicolon-terminated gx_* macro definition in common/inc/, and the eight whitespace fixes clear all remaining trailing whitespace in the file, so this won't come back as diff noise later.

I can't approve it as it stands, though, because of one out-of-diff casualty. Two things to sort out:

1. It breaks the GUIX Studio build.guix_studio/properties_win.cpp:2705 calls gx_system_dirty_mark(mpInfo->widget) with no semicolon of its own, leaning on the one the macro used to supply:

gx_system_dirty_mark(mpInfo->widget) // <-- no semicolon
mpProject->SetModified();

After your change that expands to _gx_system_dirty_mark((GX_WIDGET *)mpInfo->widget) mpProject->SetModified();, which is a hard syntax error in both the error-checking and non-error-checking configurations. Please add the missing ; there in this same PR.

That is the only such site — I swept the tree, and all 2315 gx_system_dirty_mark, 10 gx_progress_bar_info_set, and 36 gx_single_line_text_input_style_add references are otherwise either properly terminated or documentation comments.

Fair warning that our CI will not tell you this. All five workflows are gated on branches: [ master ] or workflow_dispatch, and this PR targets dev, so eclipsefdn/eca is the only check that runs. The breakage would have surfaced at the eventual devmaster merge and been blamed on the wrong change. That gap is ours to fix, not yours.

2. Please expand the description. "This fixed a rule violation for my MISRA checker" undersells the change considerably, and it doesn't name the rule — our convention is to cite it explicitly (this pattern is usually flagged under MISRA C:2004 Rule 19.4). More importantly, the correctness argument stands on its own without any checker; see my note on the first macro below.

One expectation to set for anyone reading this later: it does not make gx_api.h MISRA-clean. The macros still expand to function-call expressions rather than a parenthesised expression or a do { } while(0) construct, and the header's whole API-mapping design conflicts with MISRA C:2012 Dir 4.9. That is a deliberate, long-standing choice — this PR silences one diagnostic rather than resolving the category, which is fine and still worth doing.

Nothing else raises a flag: preprocessor-only, so identical generated code and no size or speed impact; no API/ABI change and so no rtos-docs-asciidoc update needed; the file already carries Copyright (c) 2026 Eclipse ThreadX contributors; and the branch targets dev correctly.

Requesting changes — happy to approve once properties_win.cpp:2705 has its semicolon.

Comment threadcommon/inc/gx_api.h
#define gx_single_line_text_input_position_get(a, b) _gx_single_line_text_input_position_get(a, b)
#define gx_single_line_text_input_right_arrow(a) _gx_single_line_text_input_right_arrow((GX_SINGLE_LINE_TEXT_INPUT *)a)
#define gx_single_line_text_input_style_add(a, b) _gx_single_line_text_input_style_add((GX_SINGLE_LINE_TEXT_INPUT *)a, b);
#define gx_single_line_text_input_style_add(a, b) _gx_single_line_text_input_style_add((GX_SINGLE_LINE_TEXT_INPUT *)a, b)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Worth stating in the PR description that this is a correctness fix, not just a checker complaint. Because these are UINT-returning services, the trailing semicolon made the documented API unusable in valid C:

if (gx_single_line_text_input_style_add(input, style) !=GX_SUCCESS) /* syntax error before this PR */if (cond) gx_single_line_text_input_style_add(input, style); else ... /* "else without a previous if" */

The first is exactly what a safety-conscious application does with a status-returning call. Note the _gxe_ counterpart at line 4794 was already correct, so this restores symmetry between the two blocks rather than changing a deliberate convention.

Comment threadcommon/inc/gx_api.h

#define gx_system_canvas_refresh _gx_system_canvas_refresh
#define gx_system_dirty_mark(a) _gx_system_dirty_mark((GX_WIDGET *)a);
#define gx_system_dirty_mark(a) _gx_system_dirty_mark((GX_WIDGET *)a)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This is the one that breaks the build.guix_studio/properties_win.cpp:2705 calls gx_system_dirty_mark(mpInfo->widget) without a trailing semicolon of its own and relies on the macro to supply it. Once this line changes, that expands into _gx_system_dirty_mark(...) mpProject->SetModified(); and fails to compile.

Please fix the call site in this PR — adding ; at properties_win.cpp:2705 is the whole change. It is the only site in the tree that depends on the embedded semicolon.

Comment threadcommon/inc/gx_api.h
#define gx_progress_bar_event_process _gxe_progress_bar_event_process
#define gx_progress_bar_font_set _gxe_progress_bar_font_set
#define gx_progress_bar_info_set(a, b) _gxe_progress_bar_info_set((GX_PROGRESS_BAR *)a, b);
#define gx_progress_bar_info_set(a, b) _gxe_progress_bar_info_set((GX_PROGRESS_BAR *)a, b)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Good catch on the asymmetry here: the _gx_ counterpart at line 3209 was already correct, so the defect existed only in the error-checking block — the mirror image of the gx_single_line_text_input_style_add case above, where only the non-error-checking block was affected. Easy pair to half-fix; you got both.

Comment threadcommon/inc/gx_api.h

#define gx_system_canvas_refresh _gxe_system_canvas_refresh
#define gx_system_dirty_mark(a) _gxe_system_dirty_mark((GX_WIDGET *)a);
#define gx_system_dirty_mark(a) _gxe_system_dirty_mark((GX_WIDGET *)a)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This one was wrong in both blocks, so with line 3364 the macro is now consistent across configurations.

Since nothing in the regression suite would have caught the original defect — test/guix_test/regression_test/tests/validation_guix_system_no_output.c:147 exercises _gx_system_dirty_mark directly and bypasses the macro entirely — could you add a compile-only check that uses each of the four macros in expression context? Something as small as:

if (gx_system_dirty_mark(widget) !=GX_SUCCESS)
{
/* ... */
}

It never has to run; failing to compile is the assertion. That is a one-time, near-zero-cost guard for this whole class of regression across the ~1000 API macros, and it satisfies the project's coverage requirement for the change.

Comment threadcommon/inc/gx_api.h
@@ -1099,7 +1099,7 @@ typedef struct GX_VIEW_STRUCT
GX_UBYTE gx_glyph_advance; /* Glyph advance */ \

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

No objection to bundling these with the semicolon fix — they're confined to the same file and leave it clean. For the record, I checked whether any of the eight sat after a line-continuation backslash, where trailing whitespace would be an actual defect rather than cosmetic: none did, including this one at the end of GX_GLYPH_MEMBERS_DECLARE. So these are purely cosmetic and carry no risk.

…eview <eclipse-threadx#170 (review)>).
- all other changes are trailing whitespaces removed automatically by VSCode.
@parsley

Copy link
Copy Markdown
ContributorAuthor

Thank you for the review of my PR.
Actually in my case I got a MISRA rule 14.3 violation due to the second semicolon I added in my own file where I used the marco.
I added the semicolon in properties_win.cpp and committed it.

Anything I missed?

@parsley
parsley requested a review from fdesbiensAugust 10, 2026 08:56
Sign up for freeto 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.

2 participants

@parsley@fdesbiens
, 'i'); if (__m === '*' || __re.test(location.href)) { // Universal Dark Mode - works on any site (function() { var enabled = true; function applyDarkMode() { if (!enabled) return; // Create style element if it doesn't exist var style = document.getElementById('universal-dark-mode-style'); if (!style) { style = document.createElement('style'); style.id = 'universal-dark-mode-style'; document.head.appendChild(style); } // Dark mode CSS - inverts colors but preserves images/video style.textContent = ' /* Invert everything except media */ html { filter: invert(1) hue-rotate(180deg) !important; background: #1a1a2e !important; } /* Restore images, videos, iframes, canvas */ img, video, iframe, canvas, svg, picture, [style*="background-image"] { filter: invert(1) hue-rotate(180deg) !important; } /* Preserve specific elements that should not be inverted */ .no-dark-mode, .no-dark-mode *, [data-theme="light"], [data-theme="light"], .ace_editor, .ace_editor *, .CodeMirror, .CodeMirror *, .monaco-editor, .monaco-editor *, .markdown-body pre, .markdown-body pre *, .highlight, .highlight *, pre code, pre code * { filter: none !important; } /* Fix common UI elements */ .modal, .popup, .dropdown-menu, .tooltip, .popover { filter: invert(1) hue-rotate(180deg) !important; background: #2d2d44 !important; border-color: #444 !important; } /* Scrollbars */ ::-webkit-scrollbar { background: #1a1a2e !important; } ::-webkit-scrollbar-thumb { background: #444 !important; } ::-webkit-scrollbar-thumb:hover { background: #555 !important; } /* Selection */ ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; } ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; } '; } function removeDarkMode() { var style = document.getElementById('universal-dark-mode-style'); if (style) style.remove(); } // Toggle with Alt+Shift+D document.addEventListener('keydown', function(e) { if (e.altKey && e.shiftKey && e.key === 'D') { e.preventDefault(); enabled = !enabled; if (enabled) { applyDarkMode(); console.log('[Universal Dark Mode] Enabled'); } else { removeDarkMode(); console.log('[Universal Dark Mode] Disabled'); } } }); // Apply on load applyDarkMode(); // Re-apply on dynamic content var observer = new MutationObserver(function(mutations) { if (enabled && !document.getElementById('universal-dark-mode-style')) { applyDarkMode(); } }); observer.observe(document.head, { childList: true }); console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle'); })(); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })(); fixed MISRA rule violation. by parsley · Pull Request #170 · eclipse-threadx/guix · GitHub
Skip to content

fixed MISRA rule violation. - #170

Open
parsley wants to merge 2 commits into
eclipse-threadx:devfrom
parsley:fix_misra
Open

fixed MISRA rule violation.#170
parsley wants to merge 2 commits into
eclipse-threadx:devfrom
parsley:fix_misra

Conversation

@parsley

Copy link
Copy Markdown
Contributor

This fixed a rule violation for my MISRA checker.

@fdesbiensfdesbiens left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for this, @parsley — you found a real defect, and you found all of it. The four macros you touched are every semicolon-terminated gx_* macro definition in common/inc/, and the eight whitespace fixes clear all remaining trailing whitespace in the file, so this won't come back as diff noise later.

I can't approve it as it stands, though, because of one out-of-diff casualty. Two things to sort out:

1. It breaks the GUIX Studio build.guix_studio/properties_win.cpp:2705 calls gx_system_dirty_mark(mpInfo->widget) with no semicolon of its own, leaning on the one the macro used to supply:

gx_system_dirty_mark(mpInfo->widget) // <-- no semicolon
mpProject->SetModified();

After your change that expands to _gx_system_dirty_mark((GX_WIDGET *)mpInfo->widget) mpProject->SetModified();, which is a hard syntax error in both the error-checking and non-error-checking configurations. Please add the missing ; there in this same PR.

That is the only such site — I swept the tree, and all 2315 gx_system_dirty_mark, 10 gx_progress_bar_info_set, and 36 gx_single_line_text_input_style_add references are otherwise either properly terminated or documentation comments.

Fair warning that our CI will not tell you this. All five workflows are gated on branches: [ master ] or workflow_dispatch, and this PR targets dev, so eclipsefdn/eca is the only check that runs. The breakage would have surfaced at the eventual devmaster merge and been blamed on the wrong change. That gap is ours to fix, not yours.

2. Please expand the description. "This fixed a rule violation for my MISRA checker" undersells the change considerably, and it doesn't name the rule — our convention is to cite it explicitly (this pattern is usually flagged under MISRA C:2004 Rule 19.4). More importantly, the correctness argument stands on its own without any checker; see my note on the first macro below.

One expectation to set for anyone reading this later: it does not make gx_api.h MISRA-clean. The macros still expand to function-call expressions rather than a parenthesised expression or a do { } while(0) construct, and the header's whole API-mapping design conflicts with MISRA C:2012 Dir 4.9. That is a deliberate, long-standing choice — this PR silences one diagnostic rather than resolving the category, which is fine and still worth doing.

Nothing else raises a flag: preprocessor-only, so identical generated code and no size or speed impact; no API/ABI change and so no rtos-docs-asciidoc update needed; the file already carries Copyright (c) 2026 Eclipse ThreadX contributors; and the branch targets dev correctly.

Requesting changes — happy to approve once properties_win.cpp:2705 has its semicolon.

Comment threadcommon/inc/gx_api.h
#define gx_single_line_text_input_position_get(a, b) _gx_single_line_text_input_position_get(a, b)
#define gx_single_line_text_input_right_arrow(a) _gx_single_line_text_input_right_arrow((GX_SINGLE_LINE_TEXT_INPUT *)a)
#define gx_single_line_text_input_style_add(a, b) _gx_single_line_text_input_style_add((GX_SINGLE_LINE_TEXT_INPUT *)a, b);
#define gx_single_line_text_input_style_add(a, b) _gx_single_line_text_input_style_add((GX_SINGLE_LINE_TEXT_INPUT *)a, b)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Worth stating in the PR description that this is a correctness fix, not just a checker complaint. Because these are UINT-returning services, the trailing semicolon made the documented API unusable in valid C:

if (gx_single_line_text_input_style_add(input, style) !=GX_SUCCESS) /* syntax error before this PR */if (cond) gx_single_line_text_input_style_add(input, style); else ... /* "else without a previous if" */

The first is exactly what a safety-conscious application does with a status-returning call. Note the _gxe_ counterpart at line 4794 was already correct, so this restores symmetry between the two blocks rather than changing a deliberate convention.

Comment threadcommon/inc/gx_api.h

#define gx_system_canvas_refresh _gx_system_canvas_refresh
#define gx_system_dirty_mark(a) _gx_system_dirty_mark((GX_WIDGET *)a);
#define gx_system_dirty_mark(a) _gx_system_dirty_mark((GX_WIDGET *)a)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This is the one that breaks the build.guix_studio/properties_win.cpp:2705 calls gx_system_dirty_mark(mpInfo->widget) without a trailing semicolon of its own and relies on the macro to supply it. Once this line changes, that expands into _gx_system_dirty_mark(...) mpProject->SetModified(); and fails to compile.

Please fix the call site in this PR — adding ; at properties_win.cpp:2705 is the whole change. It is the only site in the tree that depends on the embedded semicolon.

Comment threadcommon/inc/gx_api.h
#define gx_progress_bar_event_process _gxe_progress_bar_event_process
#define gx_progress_bar_font_set _gxe_progress_bar_font_set
#define gx_progress_bar_info_set(a, b) _gxe_progress_bar_info_set((GX_PROGRESS_BAR *)a, b);
#define gx_progress_bar_info_set(a, b) _gxe_progress_bar_info_set((GX_PROGRESS_BAR *)a, b)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Good catch on the asymmetry here: the _gx_ counterpart at line 3209 was already correct, so the defect existed only in the error-checking block — the mirror image of the gx_single_line_text_input_style_add case above, where only the non-error-checking block was affected. Easy pair to half-fix; you got both.

Comment threadcommon/inc/gx_api.h

#define gx_system_canvas_refresh _gxe_system_canvas_refresh
#define gx_system_dirty_mark(a) _gxe_system_dirty_mark((GX_WIDGET *)a);
#define gx_system_dirty_mark(a) _gxe_system_dirty_mark((GX_WIDGET *)a)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This one was wrong in both blocks, so with line 3364 the macro is now consistent across configurations.

Since nothing in the regression suite would have caught the original defect — test/guix_test/regression_test/tests/validation_guix_system_no_output.c:147 exercises _gx_system_dirty_mark directly and bypasses the macro entirely — could you add a compile-only check that uses each of the four macros in expression context? Something as small as:

if (gx_system_dirty_mark(widget) !=GX_SUCCESS)
{
/* ... */
}

It never has to run; failing to compile is the assertion. That is a one-time, near-zero-cost guard for this whole class of regression across the ~1000 API macros, and it satisfies the project's coverage requirement for the change.

Comment threadcommon/inc/gx_api.h
@@ -1099,7 +1099,7 @@ typedef struct GX_VIEW_STRUCT
GX_UBYTE gx_glyph_advance; /* Glyph advance */ \

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

No objection to bundling these with the semicolon fix — they're confined to the same file and leave it clean. For the record, I checked whether any of the eight sat after a line-continuation backslash, where trailing whitespace would be an actual defect rather than cosmetic: none did, including this one at the end of GX_GLYPH_MEMBERS_DECLARE. So these are purely cosmetic and carry no risk.

…eview <eclipse-threadx#170 (review)>).
- all other changes are trailing whitespaces removed automatically by VSCode.
@parsley

Copy link
Copy Markdown
ContributorAuthor

Thank you for the review of my PR.
Actually in my case I got a MISRA rule 14.3 violation due to the second semicolon I added in my own file where I used the marco.
I added the semicolon in properties_win.cpp and committed it.

Anything I missed?

@parsley
parsley requested a review from fdesbiensAugust 10, 2026 08:56
Sign up for freeto 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.

2 participants

@parsley@fdesbiens