[PATCH] D45050: [clang-tidy] New checker for not null-terminated result caused by strlen or wcslen

[Whisperity via Phabricator via cfe-commits]

whisperity requested changes to this revision.
whisperity added a comment.
This revision now requires changes to proceed.

In general, make sure the documentation page renders well in a browser.

Mostly style and phrasing stuff inline:



================
Comment at: clang-tidy/bugprone/NotNullTerminatedResultCheck.h:34-36
+  // If non-zero the target version implements _s suffixed memory and character
+  // handler functions which is safer than older versions (e.g. 'memcpy_s()').
+  const int IsSafeFunctionsAreAvailable;
----------------
More of a language or phrasing thing, but the singular/plural wording is anything but matched in this case: handler functions which **are** safer. What is a "target version" in this case? Shouldn't it be something like "target environment" or "target standard" or just simply "target"?

The variable name is also problematic. `AreSafeFunctionsAvailable` would be better.


================
Comment at: docs/clang-tidy/checks/bugprone-not-null-terminated-result.rst:20-21
+the passed third argument, which is ``size_t length``. The proper length is
+``strlen(src) + 1`` because the null terminator need an extra space. The result
+is badly not null-terminated:
+
----------------
//badly?// Also, perhaps a half sentence of explanation would be nice here:

need an extra space, thus the result is not null-terminated, which can result in undefined behaviour when the string is read.


================
Comment at: docs/clang-tidy/checks/bugprone-not-null-terminated-result.rst:39
+
+Otherwise fix-it will rewrite it to a safer function, that born before ``_s``
+suffixed functions:
----------------
That //existed//.


================
Comment at: docs/clang-tidy/checks/bugprone-not-null-terminated-result.rst:105
+
+   An integer non-zero value specifying if the target version implements ``_s``
+   suffixed memory and character handler functions which is safer than older
----------------
Why is this an integer, rather than a bool?


https://reviews.llvm.org/D45050