Skip to content

F.7 rule does not take into account the guarantees smart pointers provide. #2282

@ImaginaryExponent

Description

@ImaginaryExponent

Per the stated guideline, "F.7: For general use, take T* or T& arguments rather than smart pointers" we should avoid accepting smart pointers as function arguments. The reasoning provided is, of course, valid, but it seems to miss an important point.

A function is a contract: "Given valid input arguments the function will do whatever it is intended to do". Specifically, the issue is in "valid" of the input arguments. When a function accepts raw pointers it has no means to distinguish between non-null valid and non-null invalid arguments.

void run_task(const workload *arg)
{
 if (nullptr == arg)
  return;
...
}

This check can clearly detect one of the invalid parameter types. But it cannot protect against a dangling pointer. A very contrived example would be

workload *wl = &workload_storage.front();
workload_storage.pop_front();
run_task(wl);

Here run_task cannot know it has been provided a deleted pointer. It gets a non-null value and crashes at runtime. But there is no other check the run_task could perform. It can only rely on the caller to perform the validations. Which is perfectly safe in many cases, but not when the function is a part of some public interface.
On the other hand, if workload is already stored as, for instance, std::shared_ptr, or any other pointer-tracking type, the behavior is different:

void run_task(const std::shared_ptr<workload> &arg)
{
 if (nullptr == arg)
  return;
...
}

now the function relies on the std::shared_ptr for the guarantees. It does not affect the ownership, yes, but it actively relies on its side-effect. Now a non-null value will definitely point to a valid object in memory. The check is now necessary and sufficient for the validity of the argument.

This guarantee seems important enough to be at least mentioned as an option for the argument passing.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions