microsoft / microsoft/CsWin32

Reconsider taking SafeHandle-derived types as input parameters

Open
#125 12 comments 2 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
C#
Stars
2.5k
Forks
124
Avg merge
1d 3h
Merged PRs (30d)
9

Description

No semantically correct SafeHandle type is provided by CsWin32 when using GetCursorInfo with GetIconInfo and DrawIconEx, now that all handle parameters are SafeHandle:

var cursorInfo = new CURSORINFO { cbSize = Marshal.SizeOf(typeof(CURSORINFO)) };

if (!PInvoke.GetCursorInfo(ref cursorInfo))
    throw new Win32Exception();

//                           ↓ Options: abuse an existing SafeHandle type or declare one myself
if (!PInvoke.GetIconInfo(new WhatDoIWriteHere(cursorInfo.hCursor, ownsHandle: false), out var iconInfo))
    throw new Win32Exception();
    
// Calling DrawIconEx has the same issue.

Prior to everything becoming abstract SafeHandle, the following code worked. It's not as ergonomic as passing HCURSOR, but it is nicer than the two new options:

if (!PInvoke.GetIconInfo(new(cursorInfo.hCursor, ownsHandle: false), out var iconInfo))
    throw new Win32Exception();

I'm also not a fan of throwing away the parameter type information in this way. GetObject has parameter SafeHandle h now, for instance. A parameter like that doesn't instantly tell you what kinds of handle you can pass or how to obtain them.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start by reviewing the generated signatures and call sites for GetCursorInfo, GetIconInfo, DrawIconEx, and GetObject, focusing on how SafeHandle parameters are represented. Done requires an agreed parameter-typing design that preserves applicable handle information without making the shown calls semantically incorrect.

Written by the indexing model from the issue text.

Assessment

Tech stack
csharp
Domain
devtools
Issue type
Feature
Difficulty
5/5
Estimated time
Over a week
Activity status
Stale
Clarity
Needs clarification
Newbie friendliness
25/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.