Skip to content
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 12 additions & 12 deletions common/inc/gx_api.h
Original file line number Diff line number Diff line change
Expand Up @@ -662,7 +662,7 @@ typedef struct GX_STRING_STRUCT
#define GX_TYPE_RICH_TEXT_VIEW 144
#define GX_TYPE_GENERIC_SCROLL_WHEEL 145

/* A pseudo-type, used by Studio code generator */
/* A pseudo-type, used by Studio code generator */
#define GX_TYPE_TEMPLATE 200

/* Applications can define their own window types */
Expand Down Expand Up @@ -1027,7 +1027,7 @@ typedef GX_UBYTE GX_CHAR_CODE;
#if !defined(GX_MOUSE_MAX_RESOLUTION)
#define GX_MOUSE_MAX_RESOLUTION 32
#endif
#endif
#endif

#if defined(GX_DYNAMIC_BIDI_TEXT_SUPPORT)
#define GX_PROMPT_BIDI_RESOLVED_TEXT_INFO GX_BIDI_RESOLVED_TEXT_INFO *gx_prompt_bidi_resolved_text_info;
Expand Down Expand Up @@ -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.

GX_BYTE gx_glyph_leading; /* Pen x-pre-advance */ \
GX_UBYTE gx_glyph_width; \
GX_UBYTE gx_glyph_height;
GX_UBYTE gx_glyph_height;

/* Define the Glyph type. */
typedef struct GX_GLYPH_STRUCT
Expand All @@ -1111,15 +1111,15 @@ typedef struct GX_GLYPH_STRUCT
/* Define the Kerning Glyph type. */
typedef struct GX_KERNING_GLYPH_STRUCT
{
GX_GLYPH_MEMBERS_DECLARE
GX_CONST GX_UBYTE *gx_kerning_table; /* Kerning table */
GX_GLYPH_MEMBERS_DECLARE
GX_CONST GX_UBYTE *gx_kerning_table; /* Kerning table */
} GX_KERNING_GLYPH;
#endif

/* Define the Compressed Glyph type. */
typedef struct GX_COMPRESSED_GLYPH_STRUCT
{
GX_GLYPH_MEMBERS_DECLARE
GX_GLYPH_MEMBERS_DECLARE
USHORT gx_glyph_map_size; /* Glyph size */
} GX_COMPRESSED_GLYPH;

Expand Down Expand Up @@ -1249,7 +1249,7 @@ typedef struct GX_EVENT_STRUCT
USHORT gx_event_sender; /* ID of the event sender */
union
{
UINT gx_event_timer_id;
UINT gx_event_timer_id;
GX_POINT gx_event_pointdata;
GX_UBYTE gx_event_uchardata[4];
USHORT gx_event_ushortdata[2];
Expand Down Expand Up @@ -1356,7 +1356,7 @@ typedef struct GX_RADIAL_PROGRESS_BAR_INFO_STRUCT
GX_VALUE gx_radial_progress_bar_info_selected_brush_width; /* width of upper track. */
GX_RESOURCE_ID gx_radial_progress_bar_info_normal_brush_color; /* resource ID of color for lower track. */
GX_RESOURCE_ID gx_radial_progress_bar_info_selected_brush_color; /* resource ID of color for upper track. */

} GX_RADIAL_PROGRESS_BAR_INFO;

/* Define radial slider information structure, this structure is used to
Expand Down Expand Up @@ -3308,7 +3308,7 @@ typedef struct GX_FIXED_POINT_STRUCT
#define gx_single_line_text_input_left_arrow(a) _gx_single_line_text_input_left_arrow((GX_SINGLE_LINE_TEXT_INPUT *)a)
#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.

#define gx_single_line_text_input_style_remove(a, b) _gx_single_line_text_input_style_remove((GX_SINGLE_LINE_TEXT_INPUT *)a, b)
#define gx_single_line_text_input_style_set(a, b) _gx_single_line_text_input_style_set((GX_SINGLE_LINE_TEXT_INPUT *)a, b)
#define gx_single_line_text_input_text_color_set(a, b, c, d, e) _gx_single_line_text_input_text_color_set((GX_SINGLE_LINE_TEXT_INPUT *)a, b, c, d, e)
Expand Down Expand Up @@ -3361,7 +3361,7 @@ typedef struct GX_FIXED_POINT_STRUCT
#endif

#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.

#define gx_system_dirty_partial_add(a, b) _gx_system_dirty_partial_add((GX_WIDGET *)a, b)
#define gx_system_draw_context_get _gx_system_draw_context_get
#define gx_system_event_fold _gx_system_event_fold
Expand Down Expand Up @@ -4690,7 +4690,7 @@ UINT _gx_window_wallpaper_set(GX_WINDOW *window, GX_RESOURCE_ID wallpaper_id, GX
#define gx_progress_bar_draw _gx_progress_bar_draw
#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.

#define gx_progress_bar_pixelmap_set _gxe_progress_bar_pixelmap_set
#define gx_progress_bar_range_set _gxe_progress_bar_range_set
#if defined(GUIX_5_4_0_COMPATIBILITY)
Expand Down Expand Up @@ -4844,7 +4844,7 @@ UINT _gx_window_wallpaper_set(GX_WINDOW *window, GX_RESOURCE_ID wallpaper_id, GX
#endif

#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.

#define gx_system_dirty_partial_add(a, b) _gxe_system_dirty_partial_add((GX_WIDGET *)a, b)
#define gx_system_draw_context_get _gxe_system_draw_context_get
#define gx_system_event_fold _gxe_system_event_fold
Expand Down