Skip to content

Updated C6101 #4053

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 2 commits into from
Closed

Updated C6101 #4053

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
9511f1c
Updated C6101
MugBergerFries Jul 19, 2022
97c80f3
Updated C6101
MugBergerFries Jul 19, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 20 additions & 2 deletions docs/code-quality/c6101.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,24 @@ ms.assetid: 8546367c-5de5-479a-a231-c15c0aa89ef1
---
# C6101

> warning C6101: Returning uninitialized memory
**Warning C6101: Returning uninitialized memory**\
Example output: ```Returning uninitialized memory '*p1'. A successful path through the function does not set the named _Out_ parameter.```
Copy link
Contributor

@colin-home colin-home Jul 22, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

For consistency within the C++ documentation, a blockquote for the warning message is preferable to bold. Normally, we'd leave out the "Warning C6101" warning number prefix, and put the actual warning text, with any parameters given a metavariable name and included in italics. In this case, I'd expect the H1 to read "Warning C6101" or possibly "Code Analysis Warning 6101" depending on how it appears in VS.

The markdown for the error itself would read like so:

> Returning uninitialized memory '\**parameter-name*'. A successful path through the function does not set the named \_Out\_ parameter.

This markdown should in turn render something like:

Returning uninitialized memory '*parameter-name'. A successful path through the function does not set the named _Out_ parameter.

This styling allows for localization, which may or may not exactly match the actual localized warning message, but at least it has a chance, while the code-styled example does not.

There was a very simple style consistency pass made on these diagnostics articles when they were migrated from the Visual Studio docs to the C++ docs, but not an in-depth update. That opportunity is now.

Copy link
Contributor Author

@MugBergerFries MugBergerFries Jul 26, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Making a private PR for this file. In that PR, I'm going to keep H1 the same for now, since the name of the warning (in this case 'Returning unitialized memory') does not always show up in the error.

e.g:
Warning C28720: IsBadXXXPtr API Usage (ISBADXXXPTR_API_USAGE)
Example output: Banned API Usage: function name is insecure and has been marked deprecated.

I'll change the output markdown to match your suggestion though!


A successful path through the function does not set the named `_Out_` parameter. This message is generated based on SAL annotations that indicate that the function in question always succeeds. A function that doesn't return a success/failure indication should set all of its `_Out_` parameters because the analyzer assumes that the `_Out_` parameter is uninitialized data before the function is called, and that the function will set the parameter so that it's no longer uninitialized. If the function does indicate success/failure, then the `_Out_` parameter doesn't have to be set in the case of failure, and you can detect and avoid the uninitialized location. In either case, the objective is to avoid the reading of an uninitialized location. If the function sometimes doesn't touch an `_Out_` parameter that's subsequently used, then the parameter should be initialized before the function call and be marked with the `_Inout_` annotation, or the more explicit `_Pre_null_` or `_Pre_satisfies_()` when appropriate. "Partial success" can be handled with the `_When_` annotation. For more information, see [Using SAL Annotations to Reduce C/C++ Code Defects](../code-quality/using-sal-annotations-to-reduce-c-cpp-code-defects.md).
This message is generated based on SAL annotations that indicate that the function in question always succeeds. A function that doesn't return a success/failure indication should set all of its `_Out_` parameters because the analyzer assumes that the `_Out_` parameter is uninitialized data before the function is called, and that the function will set the parameter so that it's no longer uninitialized. If the function does indicate success/failure, then the `_Out_` parameter doesn't have to be set in the case of failure, and you can detect and avoid the uninitialized location. In either case, the objective is to avoid the reading of an uninitialized location. If the function sometimes doesn't touch an `_Out_` parameter that's subsequently used, then the parameter should be initialized before the function call and be marked with the `_Inout_` annotation, or the more explicit `_Pre_null_` or `_Pre_satisfies_()` when appropriate. "Partial success" can be handled with the `_When_` annotation. For more information, see [Using SAL Annotations to Reduce C/C++ Code Defects](../code-quality/using-sal-annotations-to-reduce-c-cpp-code-defects.md).
Copy link
Contributor

Choose a reason for hiding this comment

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

When working in the public repo, you don't get Acrolinx scores, but in the private repo, you'd find it complaining about two or three issues in this paragraph. It wouldn't hurt to see what it reports.


## Example
Copy link
Contributor

Choose a reason for hiding this comment

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

As a style issue, there should be one blank line to either side of headings, code blocks, and lists. Aim for zero reported issues from markdownlint.

The following code generates this warning:
Copy link
Contributor

Choose a reason for hiding this comment

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

Consider moving the explanation into the initial statement that introduces the example:

The following sample causes warning C6101 because the `int *p1` parameter is marked `_Out_`, but it isn't set in a code path through the function:

```cpp
void example_func(_Out_ int *p1)
{
return;
}
```
This is due to the pointer p1 not being set despite being annotated with ```_Out_```. The following code avoids this warning:
Copy link
Contributor

Choose a reason for hiding this comment

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

The next paragraph should explain how to resolve the issue:

To resolve the issue, you can set the value of the parameter. Or, if the value is always initialized before the function is called, change the SAL annotation to `_InOut_`. The following sample sets the value of the parameter to resolve the issue:

```cpp
void example_func(_Out_ int *p1)
{
*p1 = 1;
return;
}
```
Copy link
Contributor

Choose a reason for hiding this comment

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

For reasons I don't fully understand, the pull request acceptance standard used by our editors says that if there is an H2 heading, there must be at least two H2 headings. Since this PR adds an (entirely justified) Example heading, it should also have either a Remarks heading between the blockquote section with the message and the first explanatory paragraph, or the Example section should be followed by a See also section that includes links to related content. Or both. I'll often get away with just one H2 if it was that way to start with, but when you add one, the editors tend to apply the rule.