E538 Refactor `GetFileShares` by xtqqczze · Pull Request #26155 · PowerShell/PowerShell · GitHub
[go: up one dir, main page]

Skip to content

Conversation

xtqqczze
Copy link
Contributor
@xtqqczze xtqqczze commented Oct 7, 2025
  • Use unmanaged pointers instead of nint
  • Avoid pointer arithmetic, use Span instead
  • Fix PInvoke definitions
  • Avoid PtrToStructure and use blittable struct for performance.

Follow-up to: #25896

* Use unmanaged pointers instead of `nint`
* Avoid pointer arithmetic, use Span instead
* Fix PInvoke definitions
* Avoid `PtrToStructure` and use blittable struct for performance.
public ushort* remark;
}

internal static uint NetShareEnum<T>(string? servername, out T* pShareInfo, out int count) where T : unmanaged
Copy link
Collaborator

Choose a reason for hiding this comment

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

Since it is high level helper it could return just List so that we have no interop/unsafe code in call site.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We need to free the unmanaged buffer, so we cannot return just a managed type.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I considered implementing a class deriving from SafeBuffer, but this would be much more code and abstraction for a method that is only called once.

Copy link
Collaborator

Choose a reason for hiding this comment

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

If we build List in the method we can free the buffer in the method too.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If we build List in the method we can free the buffer in the method too.

This is exactly what the GetFileShares method does already :p

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.

2 participants

0