[SERVER-73911] Implement a clang-tidy check like `readability-redundant-string-cstr` but for StringData. Created: 10/Feb/23  Updated: 13/Feb/23

Status: Backlog
Project: Core Server
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Task Priority: Major - P3
Reporter: Mark Benvenuto Assignee: Backlog - Security Team
Resolution: Unresolved Votes: 0
Labels: None
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Assigned Teams:
Server Security
Participants:

 Description   

Consider adding a check for certain wasteful calls with std::string that results in unnecessary memory allocations or calls to strlen. For instance: std::string(std::string("abc").c_str()) would be waste and could be simplified as std::string("abc"). While this example is contrived, there are other likely possibilities with implicit constructor calls with StringData. Clang-tidy already has a check called readability-redundant-string-cstr that inspired me.

Potentially useful checks:
1. StringData(std::string("abc").c_str())

  • wasteful as it requires a call strlen instead of the StringData(std::string&) constructor
    2. StringData(StringData("abc").toString())
  • allocates a std::string instead of directly passing a StringData to a StringData.

I am not sure how frequent these or other wasteful patterns with StringData are in the code though. It may or may not be worth considering to adding a clang-tidy check for these.

Reference:
https://clang.llvm.org/extra/clang-tidy/checks/readability/redundant-string-cstr.html
Example of a similar check for LLVM's StringRef type.
https://github.com/llvm/llvm-project/blob/3de0bc4c3d0284354b0c0ec8ca1536ee080193e2/clang-tools-extra/clang-tidy/readability/RedundantStringCStrCheck.cpp#L162-L180



 Comments   
Comment by Alex Neben [ 11/Feb/23 ]

Hey mark.benvenuto@mongodb.com while this request seems reasonable our team is limiting the scope of clang-tidy fixes we are signing up for. We want to build a platform that other server engineers can use to write their own clang-tidy checks. We do not want all clang-tidy checks flow through our team. For that reason I am going to assign this back to you. You might be able to convince service arch to take this on?

Generated at Thu Feb 08 06:25:58 UTC 2024 using Jira 9.7.1#970001-sha1:2222b88b221c4928ef0de3161136cc90c8356a66.