Apply suggestions from code review

Co-authored-by: Fred Tingaud <[email protected]>

rules/S7128/cfamily/metadata.json

@@ -1,5 +1,5 @@
 {
-  "title": "\"strlen\" should not be called on incremented pointer",
+  "title": "\"strlen\" should not be called on incremented pointers",
   "type": "CODE_SMELL",
   "status": "ready",
   "remediation": {
@@ -8,7 +8,7 @@
   },
   "tags": [
   ],
-  "defaultSeverity": "Minor",
+  "defaultSeverity": "Major",
   "ruleSpecification": "RSPEC-7128",
   "sqKey": "S7128",
   "scope": "All",

rules/S7128/cfamily/rule.adoc

@@ -1,26 +1,26 @@
-This rule raises the issue of calls `strlen(ptr + 1)`, for which the intent is unclear.
+Calling `strlen(ptr + 1)` looks very similar to the more common pattern `strlen(ptr) + 1` and has unclear intent.
 
 == Why is this an issue?
 
 The call `strlen(ptr) + 1` is commonly used to compute the memory required to copy a string:
-the one is needed to allocate space for a null-terminator.
+the `+ 1` is needed to allocate space for a null-terminator.
 
-The very similar call `strlen(ptr + 1)` that differs by placement of the right parenthesis,
+The very similar call `strlen(ptr + 1)` that differs only by the placement of the right parenthesis,
 produces vastly different results:
 
- * non-empty string computes the length of string without the first character (reduced by one)
- * if the string is empty, traverse memory after it until the null terminator (`'\0'`) is found.
+ * non-empty string computes the length of string without the first character (`strlen(ptr) - 1`)
+ * if the string is empty, it traverses neighboring memory until a null terminator (`'\0'`) is found.
 
 The latter may lead to undefined behavior.
 
-Due above, the `strlen(ptr + 1)` calls are likely to be unintended
-and result from the type. This rule raises issues on such calls.
+In consequence, calls to `strlen(ptr + 1)` are likely to be unintended
+and the result of a typo. This rule raises issues on such calls.
 
 === Code examples
 
 ==== Noncompliant code example
 
-[source,c]
+[source,c,diff-id=1,diff-type=noncompliant]
 ----
 size_t size = strlen(ptr + 1); // Noncompliant
 ----
@@ -29,7 +29,7 @@ size_t size = strlen(ptr + 1); // Noncompliant
 
 If the use of `strlen(ptr + 1)` was an typo:
 
-[source,c]
+[source,c,diff-id=1,diff-type=compliant]
 ----
 size_t size = strlen(ptr) + 1; // Compliant
 ----
@@ -52,13 +52,13 @@ For example, it can lead to buffer overflow if the operation was used to compute
 [source,c]
 ----
 char* duplicate(char const* source) {
-  char result = (char*)malloc(strlen(source + 1)); // Should be malloc(strlen(source) + 1))
+  char* result = (char*)malloc(strlen(source + 1)); // Should be malloc(strlen(source) + 1))
   strcpy(result, source);  // Writes two characters outside of "result" buffer
   return result;
 }
 ----
 
-Buffer overflow, as other undefined behavior, has a wide range of effects.
+Buffer overflows, as other undefined behavior, have a wide range of effects.
 In many cases, the access works by accident and succeeds at writing or reading a value.
 However, it can start misbehaving at any time.
 If compilation flags, compiler, platform, or runtime environment change,