Skip to content

[215_7] fix ellipse array bounds issue (#2944)#2948

Closed
physical168 wants to merge 3 commits intoMoganLab:mainfrom
physical168:physical168/215_7/fix_ellipse_bounds
Closed

[215_7] fix ellipse array bounds issue (#2944)#2948
physical168 wants to merge 3 commits intoMoganLab:mainfrom
physical168:physical168/215_7/fix_ellipse_bounds

Conversation

@physical168
Copy link
Copy Markdown

No description provided.

@physical168
Copy link
Copy Markdown
Author

This PR addresses the crash issue when interacting with ellipses in drawing mode (#2944).

Problem:
ellipse_rep::get_control_points returned 3 control points, but initialized the abs array with only 2 values. This led to out-of-bounds access during graphical selection.

Solution:
Aligned the abs array size to 3 with values 0.0, 0.5, 1.0 to match the control point count.

Details:

  • Modified src/Graphics/Types/curve.cpp
  • Added developer documentation at devel/215_7.md

@JackYansongLi
Copy link
Copy Markdown
Contributor

Please format your code with Clang@19.

@physical168
Copy link
Copy Markdown
Author

Thanks for reviewing my code! I've just reformatted the code using Clang and pushed the update. Please check if it complies with the project's style.

@wumoin
Copy link
Copy Markdown
Contributor

wumoin commented Mar 9, 2026

I was unable to reproduce the crash by following the provided test steps.

Comment thread devel/215_7.md
Fixed a program crash issue when selecting or box-selecting an ellipse.

### Why
In the original `ellipse_rep::get_control_points`, the function informed the caller that it had 3 control points (`np=3`, two foci and one boundary point). However, the `abs` array, which represents the parameter range for each point on the curve, was hardcoded to initialize with only two elements (`array<double> (0.0, 1.0)`). This caused an out-of-bounds access on `abs[2]` during the edge-snapping retrieval or distance calculation in the `curve_box_rep::graphical_select` function (which iterates from 0 to `np` seeking `abs[i]` and `abs[(i+1)%np]`), leading to undefined behavior and infinite loops.
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

感觉这种修复方式比较临时,是否有方案对于abs访问加限制,对于不合理的访问进行兜底,而不是未定义行为导致crash

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.

4 participants