Skip to content
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.

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

Create rule S7121 Calls to c_str() should not implicitly recreate strings or string_views CPP-3435 #4391

Merged
merged 9 commits into from
Nov 7, 2024
23 changes: 23 additions & 0 deletions rules/S7121/cfamily/metadata.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
{
"title": "Calls to c_str() should not implicitly recreate strings or string_views",
"type": "CODE_SMELL",
"status": "ready",
"remediation": {
"func": "Constant\/Issue",
"constantCost": "5min"
},
"tags": [
],
"defaultSeverity": "Major",
"ruleSpecification": "RSPEC-7121",
"sqKey": "S7121",
"scope": "All",
"defaultQualityProfiles": ["Sonar way"],
"quickfix": "targeted",
tomasz-kaminski-sonarsource marked this conversation as resolved.
Show resolved Hide resolved
"code": {
"impacts": {
"MAINTAINABILITY": "MEDIUM"
},
"attribute": "EFFICIENT"
}
}
57 changes: 57 additions & 0 deletions rules/S7121/cfamily/rule.adoc
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
Unnecessary calls to `c_str()` or `data()` reduce readability and performance.

== Why is this an issue?

In the process of refactoring code from C to C++, it can easily happen that redundant calls to `c_str()`
or `data()` on standard strings or string_views get introduced when the original string could be used directly.

[source,cpp,diff-id=1,diff-type=noncompliant]
----
void takeString(std::string const& dest);

void takeStringView(std::string_view dest);

void func(std::string const& src, std::string_view srcView) {
takeString(src.c_str()); // Noncompliant: redundant copy
takeStringView(src.c_str()); // Noncompliant: redundant copy
takeStringView(srcView.data()); // Noncompliant: redundant crossing of the content
}
----

Not only is this unnecessary step a readability issue, but it will force the creation of useless intermediary strings,
or force to cross the content of a string_view to search for the null-terminator, reducing the performance.
The call to `c_str()` or `data()` should be removed.

[source,cpp,diff-id=1,diff-type=compliant]
----
void takeString(std::string const& dest);

void takeStringView(std::string_view dest);

void func(std::string const& src, std::string_view srcView) {
takeString(src); // Compliant
takeStringView(src); // Compliant
takeStringView(srcView); // Compliant
}
----

=== Exception

This rule does not raise when explicitly creating a string or a string_view from `c_str()`.

[source,cpp]
----
void takeString(std::string const& dest);

void func(std::string const& src) {
takeString(std::string(src.c_str())); // Compliant by exception
}
----

This operation could be done on purpose in the rare case where `src` is a string containing embedded null-terminators, in order to only keep the content up to the first null-terminator in `dest`.

tomasz-kaminski-sonarsource marked this conversation as resolved.
Show resolved Hide resolved
== Resources

=== Related rules

* S6231 - "std::string_view" and "std::span" parameters should be directly constructed from sequences
2 changes: 2 additions & 0 deletions rules/S7121/metadata.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
{
}
Loading