Skip to content

Removed dumb smart pointers#10

Open
JoshuaHassler wants to merge 3 commits intomasterfrom
jhassler/bugfix/remove_smart_pointers
Open

Removed dumb smart pointers#10
JoshuaHassler wants to merge 3 commits intomasterfrom
jhassler/bugfix/remove_smart_pointers

Conversation

@JoshuaHassler
Copy link
Copy Markdown

@JoshuaHassler JoshuaHassler commented Apr 3, 2018

I didn't check for memory leaks yet ¯\_(ツ)_/¯

@codecov
Copy link
Copy Markdown

codecov bot commented Apr 3, 2018

Codecov Report

Merging #10 into master will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master      #10   +/-   ##
=======================================
  Coverage   83.45%   83.45%           
=======================================
  Files           8        8           
  Lines         417      417           
=======================================
  Hits          348      348           
  Misses         69       69
Impacted Files Coverage Δ
include/octopos.h 100% <ø> (ø) ⬆️
src/octopos.cpp 82.28% <100%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1fcdd64...8b8edbe. Read the comment docs.

static std::vector<int> semids;
/*! The global list of tentacles used for communication with children */
static std::vector<std::shared_ptr<tentacle>> tentacles;
static std::vector<tentacle*> tentacles;
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Well I like smart pointers. :(

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I forget why we wanted to remove them. Probably had to do with shm


int octopOS::shmid = 0;
int octopOS::tentacle_ids[NUMMODULES];
std::vector<std::shared_ptr<tentacle>> octopOS::tentacles;
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Unsafe :(

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

This whole branch is way behind master I think

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Rewrite it in rust

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

+2

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants