Apply suggestions from code review
Co-authored-by: Fred Tingaud <95592999+frederic-tingaud-sonarsource@users.noreply.github.com>
This commit is contained in:
parent
4a041b721b
commit
76ce9bb335
@ -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",
|
"type": "CODE_SMELL",
|
||||||
"status": "ready",
|
"status": "ready",
|
||||||
"remediation": {
|
"remediation": {
|
||||||
@ -8,7 +8,7 @@
|
|||||||
},
|
},
|
||||||
"tags": [
|
"tags": [
|
||||||
],
|
],
|
||||||
"defaultSeverity": "Minor",
|
"defaultSeverity": "Major",
|
||||||
"ruleSpecification": "RSPEC-7128",
|
"ruleSpecification": "RSPEC-7128",
|
||||||
"sqKey": "S7128",
|
"sqKey": "S7128",
|
||||||
"scope": "All",
|
"scope": "All",
|
||||||
|
@ -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?
|
== Why is this an issue?
|
||||||
|
|
||||||
The call `strlen(ptr) + 1` is commonly used to compute the memory required to copy a string:
|
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:
|
produces vastly different results:
|
||||||
|
|
||||||
* non-empty string computes the length of string without the first character (reduced by one)
|
* non-empty string computes the length of string without the first character (`strlen(ptr) - 1`)
|
||||||
* if the string is empty, traverse memory after it until the null terminator (`'\0'`) is found.
|
* if the string is empty, it traverses neighboring memory until a null terminator (`'\0'`) is found.
|
||||||
|
|
||||||
The latter may lead to undefined behavior.
|
The latter may lead to undefined behavior.
|
||||||
|
|
||||||
Due above, the `strlen(ptr + 1)` calls are likely to be unintended
|
In consequence, calls to `strlen(ptr + 1)` are likely to be unintended
|
||||||
and result from the type. This rule raises issues on such calls.
|
and the result of a typo. This rule raises issues on such calls.
|
||||||
|
|
||||||
=== Code examples
|
=== Code examples
|
||||||
|
|
||||||
==== Noncompliant code example
|
==== Noncompliant code example
|
||||||
|
|
||||||
[source,c]
|
[source,c,diff-id=1,diff-type=noncompliant]
|
||||||
----
|
----
|
||||||
size_t size = strlen(ptr + 1); // 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:
|
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
|
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]
|
[source,c]
|
||||||
----
|
----
|
||||||
char* duplicate(char const* source) {
|
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
|
strcpy(result, source); // Writes two characters outside of "result" buffer
|
||||||
return result;
|
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.
|
In many cases, the access works by accident and succeeds at writing or reading a value.
|
||||||
However, it can start misbehaving at any time.
|
However, it can start misbehaving at any time.
|
||||||
If compilation flags, compiler, platform, or runtime environment change,
|
If compilation flags, compiler, platform, or runtime environment change,
|
||||||
|
Loading…
x
Reference in New Issue
Block a user