Skip to content

Bug Fixes#320

Closed
starlightretailceo wants to merge 3 commits intoNVIDIA:mainfrom
STARLIGHTRETAIL:main
Closed

Bug Fixes#320
starlightretailceo wants to merge 3 commits intoNVIDIA:mainfrom
STARLIGHTRETAIL:main

Conversation

@starlightretailceo
Copy link
Copy Markdown

This pull request fixes a potential bug in the dxcore_query_adapter_driverstore function to ensure proper handling of string termination during conversion.

Bug Fix:

  • Updated the wcstombs function call to use outputSize + 1 instead of outputSize to account for the NUL terminator, ensuring the resulting string is always properly terminated.

Comment thread src/dxcore.c
* length equals `outputSize`. Use the full buffer size here to ensure
* the resulting string is always NUL terminated.
*/
wcstombs(*ppDriverStorePath, pOutput, outputSize + 1);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

As a matter of interest, is this something that you've run into?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

cc @jbujak

Comment thread src/dxcore.c
* room for the terminating NUL byte. Passing only `outputSize` could
* prevent the terminator from being written when the converted string
* length equals `outputSize`. Use the full buffer size here to ensure
* the resulting string is always NUL terminated.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
* the resulting string is always NUL terminated.
* the resulting string is always NULL terminated.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Unresolved as this has not been addressed.

Comment thread src/dxcore.c
/*
* wcstombs() expects the size of the destination buffer in bytes. The
* buffer allocated above is `outputSize + 1` bytes long, which includes
* room for the terminating NUL byte. Passing only `outputSize` could
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
* room for the terminating NUL byte. Passing only `outputSize` could
* room for the terminating NULL byte. Passing only `outputSize` could

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Unresolved as this has not been addressed.

@elezar
Copy link
Copy Markdown
Member

elezar commented Oct 1, 2025

Also, please sign-off your commit as per the DCO.

@cdesiniotis
Copy link
Copy Markdown
Contributor

@starlightretailceo your contribution is appreciated, but we cannot accept it in its current form.

  1. You have not signed your commit(s) per the DCO.
  2. The PR adds three commits when it should only be adding one commit.
  3. The code you modified enables GPU support on WSL2. This is a platform we do not regularly test / have automated test coverage for, so we are only in a position to accept critical changes (e.g. verified bug fixes).

If you are able to reproduce a bug on a WSL2 machine, please file an issue first with as much detail as you can. We can iterate from there and potentially revisit this bug. For now I am closing.

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.

3 participants