-
Notifications
You must be signed in to change notification settings - Fork 0
Open
Labels
bugSomething isn't workingSomething isn't working
Description
Description
The Windows implementation of getSymbol in ort/library_windows.go uses an unnecessary double conversion through unsafe.Pointer that is error-prone.
Current Code
File: ort/library_windows.go:24
func getSymbol(handle uintptr, symbol string) (uintptr, error) {
proc, err := windows.GetProcAddress(windows.Handle(handle), symbol)
if err != nil {
return 0, err
}
return uintptr(unsafe.Pointer(proc)), nil // Double conversion
}Issue
The double conversion uintptr(unsafe.Pointer(proc)) is unnecessary because:
windows.GetProcAddressreturns*byte- This can be converted directly to
uintptr - The intermediate
unsafe.Pointerstep adds no safety
Proposed Fix
func getSymbol(handle uintptr, symbol string) (uintptr, error) {
proc, err := windows.GetProcAddress(windows.Handle(handle), symbol)
if err != nil {
return 0, err
}
// Direct conversion - GetProcAddress returns *byte which is uintptr-compatible
return uintptr(proc), nil
}Impact
- Severity: Low (current code works, but is unnecessarily complex)
- Priority: Medium (should fix before v1.0)
- Risk: Minimal (simplification of existing working code)
Testing
- Verify Windows builds still compile
- Run existing tests on Windows platform
- No behavior change expected
Related
- Identified in PR [ENH] Implement ONNX Runtime environment management #15 review
- Part of code quality improvements before v1.0
Acceptance Criteria
- Remove unnecessary
unsafe.Pointerintermediate conversion - Add comment explaining why direct conversion is safe
- Verify Windows tests pass
- Consider adding Windows-specific test if none exist
Reactions are currently unavailable
Metadata
Metadata
Assignees
Labels
bugSomething isn't workingSomething isn't working