Skip to content

Conversation

@xsscx
Copy link
Member

@xsscx xsscx commented Dec 31, 2025

Fixes #390

Pull Request Checklist

  • Have you followed the guidelines in Contributing document?
  • Have you checked to ensure there aren't other open Pull Requests for the same change?
  • Have you built your Pull Request locally with the Build Instructions?
  • Have you added or updated relevant tests?
  • Have you added or updated relevant docs?

@xsscx xsscx requested a review from ChrisCoxArt December 31, 2025 15:13
Copilot AI review requested due to automatic review settings December 31, 2025 15:13
@xsscx xsscx added PR Pull Request Security Security Related Merge Ready Maintainer indicates Merge Ready labels Dec 31, 2025
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR attempts to fix undefined behavior (UB) reported in issue #390 related to unsafe downcasting in CIccSingleSampledeCurveXml(). The changes primarily improve code style by replacing C-style casts with static_cast, removing unnecessary parentheses, and adding braces to else clauses for consistency. However, the PR does not actually resolve the underlying undefined behavior issue.

Key changes:

  • Replaced C-style casts with static_cast for three curve type conversions
  • Removed unnecessary parentheses around conditional expressions
  • Added braces to the final else clause for consistency with K&R style

@xsscx
Copy link
Member Author

xsscx commented Dec 31, 2025

Maintainer Note

Wed Dec 31 05:08:17 PM UTC 2025

Copilot review has been adjusted.

@xsscx
Copy link
Member Author

xsscx commented Jan 1, 2026

Summary

Thu Jan 1 03:04:11 AM UTC 2026

This PR does not solve the issues.

  • neither does the Copilot suggestion

The Issue appears to require a deeper refactor across multiple files.

@xsscx xsscx closed this Jan 1, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Merge Ready Maintainer indicates Merge Ready PR Pull Request Security Security Related

Projects

None yet

Development

Successfully merging this pull request may close these issues.

TC in ToXmlCurve() at IccXML/IccLibXML/IccMpeXml.cpp:1032:41

3 participants