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

[WIP] Modernization/migrate to smart pointers #49

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion srecord/input/filter/message/adler16.cc
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ srecord::input_filter_message_adler16::process(const memory &input,
// lowest address to highest. (Holes are ignored, not filled,
// warning already issued.)
//
memory_walker_adler16::pointer w = memory_walker_adler16::create();
auto w = std::make_shared<memory_walker_adler16>();
input.walk(w);
uint16_t adler = w->get();

Expand Down
4 changes: 2 additions & 2 deletions srecord/input/filter/message/adler32.cc
Original file line number Diff line number Diff line change
Expand Up @@ -56,8 +56,8 @@ srecord::input_filter_message_adler32::process(const memory &input,
// lowest address to highest. (Holes are ignored, not filled,
// warning issued already.)
//
memory_walker_adler32::pointer w =
memory_walker_adler32::create();
auto w = std::make_shared<memory_walker_adler32>();
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Definitely keen to move to smart pointers. I should have picked these up when I moved from boost to std share pointers elsewhere.

However, I'd rather move the creation of the smart pointer into the create method in the class(es). My worry is that someone using libsrecord won't get the benefit or more importantly may mix smart and non-smart pointers if they are using base and library code from the project.


input.walk(w);
uint32_t adler = w->get();

Expand Down
7 changes: 0 additions & 7 deletions srecord/memory/walker/adler16.cc
Original file line number Diff line number Diff line change
Expand Up @@ -21,13 +21,6 @@
#include <srecord/output.h>


srecord::memory_walker_adler16::pointer
srecord::memory_walker_adler16::create()
{
return pointer(new srecord::memory_walker_adler16());
}


void
srecord::memory_walker_adler16::observe(uint32_t, const void *data,
int length)
Expand Down
22 changes: 6 additions & 16 deletions srecord/memory/walker/adler16.h
Original file line number Diff line number Diff line change
Expand Up @@ -34,27 +34,11 @@ class memory_walker_adler16:
public memory_walker
{
public:
typedef std::shared_ptr<memory_walker_adler16> pointer;

/**
* The destructor.
*/
~memory_walker_adler16() override = default;

private:
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This inclusion of private here is intentional. The idea is that the headers are written from a template and in a very structured way. Rather than mysteriously move the constructor to the bottom of the header, it's left in the usual spot as a prompt (to the lib user especially) that the create method should be used.

/**
* The default constructor. It is private on purpose, use the
* #create method instead.
*/
memory_walker_adler16() = default;

public:
/**
* The create class method is used to create new dynamically
* allocated instances of this class.
*/
static pointer create();

/**
* The get method is used to get the ADLER16 checksum once all memory
* chunks have been processed by calls to our observe method.
Expand Down Expand Up @@ -82,6 +66,12 @@ class memory_walker_adler16:
* The assignment operator.
*/
memory_walker_adler16 &operator=(const memory_walker_adler16 &) = delete;

/**
* The default constructor. It is private on purpose, use the
* #create method instead.
*/
memory_walker_adler16() = default;
};

};
Expand Down
7 changes: 0 additions & 7 deletions srecord/memory/walker/adler32.cc
Original file line number Diff line number Diff line change
Expand Up @@ -21,13 +21,6 @@
#include <srecord/output.h>


srecord::memory_walker_adler32::pointer
srecord::memory_walker_adler32::create()
{
return pointer(new srecord::memory_walker_adler32());
}


void
srecord::memory_walker_adler32::observe(uint32_t, const void *data,
int length)
Expand Down
22 changes: 6 additions & 16 deletions srecord/memory/walker/adler32.h
Original file line number Diff line number Diff line change
Expand Up @@ -34,27 +34,11 @@ class memory_walker_adler32:
public memory_walker
{
public:
typedef std::shared_ptr<memory_walker_adler32> pointer;

/**
* The destructor.
*/
~memory_walker_adler32() override = default;

private:
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is intentional, same as in adler16.h above.

/**
* The default constructor. It is private on purpose, use the
* #create method instead.
*/
memory_walker_adler32() = default;

public:
/**
* The create class method is used to create new dynamically
* allocated instances of this class.
*/
static pointer create();

/**
* The get method is used to get the ADLER32 checksum once all memory
* chunks have been processed by calls to our observe method.
Expand Down Expand Up @@ -82,6 +66,12 @@ class memory_walker_adler32:
* The assignment operator.
*/
memory_walker_adler32 &operator=(const memory_walker_adler32 &) = delete;

/**
* The default constructor. It is private on purpose, use the
* #create method instead.
*/
memory_walker_adler32() = default;
};

};
Expand Down