Shape Healing - Prevent crash on non-manifold wires in ShapeAnalysis_FreeBounds

[davidgilkaufman]

Fixes #1330

New test case: bugs/heal/bug1330

ShapeAnalysis_FreeBounds::connectWiresToWiresImpl updates internal data structures sewd, lwire, and aSel for the current loop iteration before checking sewd->NbEdges() > 0 and potentially skipping the iteration based on the result. If it skips the iteration, it correctly clears sewd and keeps a log of having processed the wire in aSel, but it does not appropriately reset lwire. I have added a new test case bugs/heal/bug1330, in which this mistake causes it to fail to set done = true after the loop, and later access invalid memory as a result.

Fix: only update lwire if the loop iteration is not skipped.

[dpasukhi]

Dear @davidgilkaufman

Thank you for your patch. To proceed with integration, please complete signing CLA process:
The links: Contribution Guide
Or https://dev.opencascade.org/get_involved

In case if you already have signed CLA, please share ID that OCCT team shared with your by email after accepting of CLA.

[davidgilkaufman]

The CLA submission website was down yesterday. but I have now signed it. Thanks for reminding me. I'll post the ID when it is sent to me.

[dpasukhi]

Thank you for update!
dev.opencascade.org and forum soon will be discontinued in the way where interaction will be without forum. And I periodically put the forum on the maitance mode to redirect people to other resources. Just to avoid any lost, I created the public archive, but I still plan to preserve original web site, but without any accounts and personal info in the future.

Later the CLA will be submittable through the email, my apologies for the complexity :(

More details: https://dev.opencascade.org/content/important-notice-upcoming-changes-devopencascadeorg-forum-and-website

[pieterhijma]

I've checked the issue (which I can confirm) and this code and it looks good to me.

This is linked to a PR in FreeCAD with workarounds for this issue. It would be good to have version guards for that workaround, so would it be possible for this PR to get added to a release? I assume it depends on the CLA verification.

[dpasukhi]

I will ping manager who responsible for CLAs, My apologies for the detays. It is not me, who processing them :*(

[dpasukhi]

@davidgilkaufman could you please write up the Telephone + Fax: ?
It is obligated fields, we not use that info for any marketing needs. It is only required by our legal team :(

I can pre-approve the CLA until the manager will officially register it.
After that I will modify your PR with GTest instead of Draw test ;) other than that I will not modify your commit

[davidgilkaufman]

I've re-submitted the form with my phone number filled out

[davidgilkaufman]

This is linked to a PR in FreeCAD with workarounds for this issue. It would be good to have version guards for that workaround, so would it be possible for this PR to get added to a release? I assume it depends on the CLA verification.

Can it be added to a release now?

We chose not to merge the FreeCAD PR at today's CAM developer meeting because we want to know the fix's release first to make proper version guards, but we plan to merge it regardless at the same meeting next week. It would be nice if we could be sure of the fix OCCT version by then.

[dpasukhi]

Yes, it will be in 8.0.1

I assume FreeCAD using only official release, and it will be only in beginning of august.

[dpasukhi]

CLA ID: 1147

[dpasukhi]

Thank you for your integration!