Updated C6101 #4053
Updated C6101 #4053
Conversation
Added an example, matched formatting to my other PRs, added an example error output
|
@MugBergerFries : Thanks for your contribution! The author(s) have been notified to review your proposed change. |
|
Docs Build status updates of commit 9511f1c:
|
| File | Status | Preview URL | Details |
|---|---|---|---|
| docs/code-quality/c6101.md | Details |
docs/code-quality/c6101.md
- Line 17, Column 1: [Warning: multiple-h1s - See documentation]
Multiple H1s(H1 'Example') are not allowed. You can only have one top-level heading.
For more details, please refer to the build report.
If you see build warnings/errors with permission issues, it might be due to single sign-on (SSO) enabled on Microsoft's GitHub organizations. Please follow instructions here to re-authorize your GitHub account to Docs Build.
Note: Broken links written as relative paths are included in the above build report. For broken links written as absolute paths or external URLs, see the broken link report.
Note: Your PR may contain errors or warnings unrelated to the files you changed. This happens when external dependencies like GitHub alias, Microsoft alias, cross repo links are updated. Please use these instructions to resolve them.
For any questions, please:
- Try searching the docs.microsoft.com contributor guides
- Post your question in the Docs support channel
Changed Example to a Header 2
|
@MugBergerFries : Thanks for your contribution! The author(s) have been notified to review your proposed change. |
|
Docs Build status updates of commit 97c80f3: ✅ Validation status: passed
For more details, please refer to the build report. Note: Broken links written as relative paths are included in the above build report. For broken links written as absolute paths or external URLs, see the broken link report. For any questions, please:
|
PRMerger Results
|
|
@corob-msft Can you review this PR? IMPORTANT: When this content is ready to merge, you must add #label:"aq-pr-triaged" |
There was a problem hiding this comment.
@MugBergerFries Thanks for taking this task on. We have a C++-specific style guide in the repo, Metadata and Markdown Template for C++ Docs, that calls out some but not all of the idiosyncratic styles we've adopted. Other issues will be easier to spot and fix if you use the private repo for your updates, as I sent instructions for separately.
| @@ -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.``` | |||
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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). |
There was a problem hiding this comment.
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.
| 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). | ||
|
|
||
| ## Example |
There was a problem hiding this comment.
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.
| 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). | ||
|
|
||
| ## Example | ||
| The following code generates this warning: |
There was a problem hiding this comment.
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:
| return; | ||
| } | ||
| ``` | ||
| This is due to the pointer p1 not being set despite being annotated with ```_Out_```. The following code avoids this warning: |
There was a problem hiding this comment.
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:
| *p1 = 1; | ||
| return; | ||
| } | ||
| ``` |
There was a problem hiding this comment.
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.
|
Closing, since this PR was moved to the private repo for preview purposes as https://github.com/MicrosoftDocs/cpp-docs-pr/pull/4429 |
|
Docs Build status updates of commit 97c80f3: ✅ Validation status: passed
For more details, please refer to the build report. Note: Broken links written as relative paths are included in the above build report. For broken links written as absolute paths or external URLs, see the broken link report. For any questions, please:
|
PRMerger Results
|
Added an example, matched formatting to my other PRs, added an example error output. This is a continuation of the Static Code Analysis documentation improvement efforts I've been working on in the /MicrosoftDocs/windows-driver-docs/windows-driver-docs-pr/devtest repo