Skip to content

fixed MISRA rule violation. - #170

Open
parsley wants to merge 1 commit into
eclipse-threadx:devfrom
parsley:fix_misra
Open

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

Conversation

@parsley

@parsley parsley commented Jul 27, 2026

Copy link
Copy Markdown
Contributor

This fixed a rule violation for my MISRA checker.

@fdesbiens fdesbiens 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 thread common/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 thread common/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 thread common/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 thread common/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 thread common/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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants