Skip to content

Commit 8e106a4

Browse files
committed
Merge rules R.12 and R.13; explain about RAII factories.
R.12 and R.13 were both written for a C-style world where the idea was to get raw resource pointers and then quickly "give" them to RAII types. The modern C++ approach is actually to use RAII types exclusively, and never handle raw resource pointers even for an instant. R.13 was kind of getting there, in that it knew `make_shared` existed; but we can strengthen the rule by just saying "Please, use `make_shared`." "Raw pointers are like raw meat: don't touch them with your hands." Also update for C++17: it's no longer true that `shared_ptr<T>(new T())` can ever cause a leak due to interleaved evaluations. But it's still a nasty habit. Use `make_shared`.
1 parent 9928c95 commit 8e106a4

File tree

1 file changed

+44
-43
lines changed

1 file changed

+44
-43
lines changed

CppCoreGuidelines.md

+44-43
Original file line numberDiff line numberDiff line change
@@ -9010,8 +9010,7 @@ Here, we ignore such cases.
90109010

90119011
* [R.10: Avoid `malloc()` and `free()`](#Rr-mallocfree)
90129012
* [R.11: Avoid calling `new` and `delete` explicitly](#Rr-newdelete)
9013-
* [R.12: Immediately give the result of an explicit resource allocation to a manager object](#Rr-immediate-alloc)
9014-
* [R.13: Perform at most one explicit resource allocation in a single expression statement](#Rr-single-alloc)
9013+
* [R.12: Instead of manual resource allocation, use factory functions that return RAII objects](#Rr-immediate-alloc)
90159014
* [R.14: Avoid `[]` parameters, prefer `span`](#Rr-ap)
90169015
* [R.15: Always overload matched allocation/deallocation pairs](#Rr-pair)
90179016

@@ -9365,73 +9364,75 @@ If you have a naked `new`, you probably need a naked `delete` somewhere, so you
93659364

93669365
(Simple) Warn on any explicit use of `new` and `delete`. Suggest using `make_unique` instead.
93679366

9368-
### <a name="Rr-immediate-alloc"></a>R.12: Immediately give the result of an explicit resource allocation to a manager object
9367+
### <a name="Rr-immediate-alloc"></a>R.12: Instead of manual resource allocation, use factory functions that return RAII objects
93699368

93709369
##### Reason
93719370

93729371
If you don't, an exception or a return may lead to a leak.
93739372

9374-
##### Example, bad
9373+
##### Example
93759374

9376-
void f(const string& name)
9375+
void bad(const char *name)
93779376
{
9378-
FILE* f = fopen(name, "r"); // open the file
9379-
vector<char> buf(1024);
9380-
auto _ = finally([f] { fclose(f); }); // remember to close the file
9377+
FILE *f = fopen(name, "r");
9378+
std::vector<char> buf(1024);
93819379
// ...
93829380
}
93839381

9384-
The allocation of `buf` may fail and leak the file handle.
9385-
9386-
##### Example
9382+
If `buf`'s constructor throws, then `FILE *f` will not be `fclose`d — a resource leak.
9383+
Prefer to use an RAII object such as `std::ifstream`.
93879384

9388-
void f(const string& name)
9385+
void good(const char *name)
93899386
{
9390-
ifstream f{name}; // open the file
9391-
vector<char> buf(1024);
9387+
std::ifstream myfile(name);
9388+
std::vector<char> buf(1024);
93929389
// ...
93939390
}
93949391

9395-
The use of the file handle (in `ifstream`) is simple, efficient, and safe.
9396-
9397-
##### Enforcement
9398-
9399-
* Flag explicit allocations used to initialize pointers (problem: how many direct resource allocations can we recognize?)
9400-
9401-
### <a name="Rr-single-alloc"></a>R.13: Perform at most one explicit resource allocation in a single expression statement
9402-
9403-
##### Reason
9404-
9405-
If you perform two explicit resource allocations in one statement, you could leak resources because the order of evaluation of many subexpressions, including function arguments, is unspecified.
9406-
94079392
##### Example
94089393

9409-
void fun(shared_ptr<Widget> sp1, shared_ptr<Widget> sp2);
9410-
9411-
This `fun` can be called like this:
9412-
9413-
// BAD: potential leak
9414-
fun(shared_ptr<Widget>(new Widget(a, b)), shared_ptr<Widget>(new Widget(c, d)));
9394+
void bad() {
9395+
return func(
9396+
std::shared_ptr<Widget>(new Widget(a, b)),
9397+
std::shared_ptr<Gadget>(new Gadget(c, d))
9398+
);
9399+
}
94159400

9416-
This is exception-unsafe because the compiler may reorder the two expressions building the function's two arguments.
9417-
In particular, the compiler can interleave execution of the two expressions:
9418-
Memory allocation (by calling `operator new`) could be done first for both objects, followed by attempts to call the two `Widget` constructors.
9419-
If one of the constructor calls throws an exception, then the other object's memory will never be released!
9401+
Prior to C++17, it was possible for this code to evaluate `new Widget(a, b)`,
9402+
followed by `new Gadget(c, d)`, followed by the constructors of both `shared_ptr`s.
9403+
If either `new Gadget` or the constructor of the first `shared_ptr` were to throw
9404+
an exception, an allocation would be leaked.
94209405

9421-
This subtle problem has a simple solution: Never perform more than one explicit resource allocation in a single expression statement.
9422-
For example:
9406+
`shared_ptr<Widget>` is an RAII type, but `Widget*` is not; therefore we should
9407+
not be making a direct call to `new`, which returns `Widget*`. The best solution
9408+
is to use the factory function `std::make_shared`, which returns an RAII object
9409+
without ever exposing the non-RAII type `Widget*` to our code.
94239410

9424-
shared_ptr<Widget> sp1(new Widget(a, b)); // Better, but messy
9425-
fun(sp1, new Widget(c, d));
9411+
void good() {
9412+
return func(
9413+
std::make_shared<Widget>(a, b),
9414+
std::make_shared<Gadget>(c, d)
9415+
);
9416+
}
94269417

9427-
The best solution is to avoid explicit allocation entirely use factory functions that return owning objects:
9418+
If there is no factory wrapper for your RAII type yet, write one!
94289419

9429-
fun(make_shared<Widget>(a, b), make_shared<Widget>(c, d)); // Best
9420+
void bad(const char *name)
9421+
{
9422+
FILE *f = fopen(name, "r"); // bad
9423+
AutoFclosingFile myfile(f);
9424+
// ...
9425+
}
94309426

9431-
Write your own factory wrapper if there is not one already.
9427+
void good(const char *name)
9428+
{
9429+
AutoFclosingFile myfile = my::open_file(name, "r");
9430+
// ...
9431+
}
94329432

94339433
##### Enforcement
94349434

9435+
* Flag explicit allocations used to initialize pointers (problem: how many direct resource allocations can we recognize?)
94359436
* Flag expressions with multiple explicit resource allocations (problem: how many direct resource allocations can we recognize?)
94369437

94379438
### <a name="Rr-ap"></a>R.14: Avoid `[]` parameters, prefer `span`

0 commit comments

Comments
 (0)