Use std::unique_ptr (C++11) where possible #11043

practicalswift · 2017-08-14T14:42:22Z

Use std::unique_ptr (C++11) where possible.

Rationale:

Avoid resource leaks (specifically: forgetting to delete an object created using new)
Avoid undefined behaviour (specifically: double delete:s)

Note to reviewers: Please let me know if I've missed any obvious std::unique_ptr candidates. Hopefully this PR should cover all the trivial cases.

sipa · 2017-08-14T18:37:54Z

src/policy/fees.cpp

-            feeStats = fileFeeStats.release();
-            shortStats = fileShortStats.release();
-            longStats = fileLongStats.release();
+            feeStats.reset(fileFeeStats.release());


How about

feeStats = std::move(fileFeeStats); shortStats = std::move(fileShortStats); longStats = std::move(fileLongStats);

practicalswift · 2017-08-14T20:22:29Z

@sipa Nice catch! Fixed! :-)

ryanofsky

It would be nice to replace all the new

std::unique_ptr<ClassName>(new ClassName(...))

occurrences this is adding with simpler

make_unique<ClassName>(...)

calls.

std::make_unique isn't available until c++14, so we can't use it yet, but it is easy enough to write a substitute. For example (from #10973):

//! Substitute for C++14 std::make_unique.
template <typename T, typename... Args>
std::unique_ptr<T> MakeUnique(Args&&... args)
{
    return std::unique_ptr<T>(new T(std::forward<Args>(args)...));
}

ryanofsky

utACK 5a92b9d89509fa17ea341da3794b34ac330669a9

ryanofsky · 2017-08-14T21:33:50Z

src/wallet/db.cpp

@@ -487,7 +487,7 @@ bool CDB::Rewrite(CWalletDBWrapper& dbw, const char* pszSkip)
                std::string strFileRes = strFile + ".rewrite";
                { // surround usage of db with extra {}
                    CDB db(dbw, "r");
-                    Db* pdbCopy = new Db(env->dbenv.get(), 0);
+                    std::unique_ptr<Db> pdbCopy = std::unique_ptr<Db>(new Db(env->dbenv.get(), 0));


In commit "Use unique_ptr for pdbCopy (Db)"

Is this fixing a potential resource / memory leak? It seems like the pointer was only being freed conditionally below. Maybe update commit description to say how this is changing behavior, if it is changing behavior.

ryanofsky · 2017-08-14T21:43:02Z

src/test/test_bitcoin.h

@@ -50,7 +50,7 @@ struct BasicTestingSetup {
 */
 class CConnman;
 struct TestingSetup: public BasicTestingSetup {
-    CCoinsViewDB *pcoinsdbview;
+    std::unique_ptr<CCoinsViewDB> pcoinsdbview;


In commit "Use unique_ptr for pcoinscatcher/pcoinsdbview/pcoinsTip/pblocktree"

Can this variable just be deleted like the one in rpcnestedtests.h, since it is shadowing a global with the same name? If it can't be deleted, it would be good to have a comment noting that it does shadow the global (and maybe explaining why).

laanwj · 2017-08-15T07:47:44Z

Avoid resource leaks (specifically: forgetting to delete an object created using new)

Just want to add that this is not a panacea. In a few places I've explicitly used manual pointer allocation/deallocation instead of unique_ptr (for globals) so to explicitly control the lifespan, because the order of deallocation matters. So in some of these cases, forgetting explicit .reset() is as bad as leaking and the 'parachute' that automatic deallocation provides isn't worth anything (could even result in spurious crashes at shutdown).

bytting · 2017-08-15T07:56:41Z

I like the concept, and encourage anyone who wants more meat on the bone to read Sutter's Mill articles about smart pointers

https://herbsutter.com/gotw/

practicalswift · 2017-08-15T08:02:07Z

@laanwj Absolutely true! My goal with this PR is to address only the trivial/safe cases :-)

I know about the dependency on deconstruction order related to LogPrintStr(…) (FILE* fileout, boost::mutex* mutexDebugLog and std::list<std::string>* vMsgsBeforeOpenLog) - so these were intentionally excluded. Were you thinking about to those when mentioning spurious crashes at shutdown? :-)

laanwj · 2017-08-15T08:04:24Z

Were you thinking about to those when mentioning spurious crashes at shutdown? :-)

No, I'll comment inline.

laanwj · 2017-08-15T08:05:25Z

src/httprpc.cpp

-        RPCUnsetTimerInterface(httpRPCTimerInterface);
-        delete httpRPCTimerInterface;
-        httpRPCTimerInterface = 0;
+        RPCUnsetTimerInterface(httpRPCTimerInterface.get());


add httpRPCTimerInterface.reset() after this

@laanwj Thanks for reviewing! Fixed!

laanwj · 2017-08-15T08:10:09Z

src/wallet/test/wallet_test_fixture.cpp

-    UnregisterValidationInterface(pwalletMain);
-    delete pwalletMain;
-    pwalletMain = nullptr;
+    UnregisterValidationInterface(pwalletMain.get());


add pwalletMain.reset() (this is not part of the WalletTestingSetup but a global, so needs to be cleaned up here and now)

Thanks! Fixed!

laanwj · 2017-08-15T08:14:16Z

I found two in a cursory look-over. There might be more instances where explicit .reset is important (where there was an explicit delete before), please be careful in reviewing this.

LogPrintStr(…) (FILE* fileout, boost::mutex* mutexDebugLog and std::liststd::string* vMsgsBeforeOpenLog)
so these were intentionally excluded

Agree with leaving these changes out for that "magic", just too easy to inadvertently break something there.

practicalswift · 2017-08-15T08:30:04Z

@ryanofsky Thanks for reviewing! Great feedback - all items addressed.

ryanofsky

utACK 6b7863325b74a6b45aa77a1fc9f3586520783f5b. Changes since previous were just all the suggested changes (restoring some explicit deletes instead of relying on destructors, removing pcoinsdbview shadowing, adding MakeUnique) plus rebasing.

practicalswift · 2017-08-16T14:21:05Z

Rebased!

laanwj · 2017-08-22T07:43:20Z

Needs rebase again as #11007 was a subset of this.

practicalswift · 2017-08-22T08:04:58Z

@laanwj Done! :-)

* pcoinscatcher (CCoinsViewErrorCatcher) * pcoinsdbview (CCoinsViewDB) * pcoinsTip (CCoinsViewCache) * pblocktree (CBlockTreeDB) * Remove variables shadowing pcoinsdbview

@ryanofsky

From @ryanofsky:s bitcoin#10973. Thanks!

practicalswift · 2017-11-09T15:54:11Z

Rebased! :-)

laanwj · 2017-11-09T20:28:57Z

Looks good to me now, utACK a357293

a357293 Use MakeUnique<Db>(...) (practicalswift) 3e09b39 Use MakeUnique<T>(...) instead of std::unique_ptr<T>(new T(...)) (practicalswift) 8617989 Add MakeUnique (substitute for C++14 std::make_unique) (practicalswift) d223bc9 Use unique_ptr for pcoinscatcher/pcoinsdbview/pcoinsTip/pblocktree (practicalswift) b45c597 Use unique_ptr for pdbCopy (Db) and fix potential memory leak (practicalswift) 29ab96d Use unique_ptr for dbenv (DbEnv) (practicalswift) f72cbf9 Use unique_ptr for pfilter (CBloomFilter) (practicalswift) 8ccf1bb Use unique_ptr for sem{Addnode,Outbound} (CSemaphore) (practicalswift) 73db063 Use unique_ptr for upnp_thread (boost::thread) (practicalswift) 0024531 Use unique_ptr for dbw (CDBWrapper) (practicalswift) fa6d122 Use unique_ptr:s for {fee,short,long}Stats (TxConfirmStats) (practicalswift) 5a6f768 Use unique_ptr for httpRPCTimerInterface (HTTPRPCTimerInterface) (practicalswift) 860e912 Use unique_ptr for pwalletMain (CWallet) (practicalswift) Pull request description: Use `std::unique_ptr` (C++11) where possible. Rationale: 1. Avoid resource leaks (specifically: forgetting to `delete` an object created using `new`) 2. Avoid undefined behaviour (specifically: double `delete`:s) **Note to reviewers:** Please let me know if I've missed any obvious `std::unique_ptr` candidates. Hopefully this PR should cover all the trivial cases. Tree-SHA512: 9fbeb47b800ab8ff4e0be9f2a22ab63c23d5c613a0c6716d9183db8d22ddbbce592fb8384a8b7874bf7375c8161efb13ca2197ad6f24b75967148037f0f7b20c

a8b5d20 Reset pblocktree before deleting LevelDB file (Sjors Provoost) Pull request description: #11043 repaced: ``` delete pblocktree; pblocktree = new CBlockTreeDB(nBlockTreeDBCache, false, fReset); ``` With: ``` pblocktree.reset(new CBlockTreeDB(nBlockTreeDBCache, false, fReset)); ``` This is problematic because `new CBlockTreeDB` tries to delete the existing file, which will fail with `LOCK: already held by process` if it's still open. That's the case for QT. When QT finds a problem with the index it will ask the user if they want to reindex. At that point it has already opened `blocks/index`. It then runs this [while loop](https://github.com/bitcoin/bitcoin/blob/v0.16.0rc3/src/init.cpp#L1415) again with `fReset = 1`, resulting in the above error. This change makes that error go away, presumably because `reset()` without an argument closes the file. Tree-SHA512: fde8b546912f6773ac64da8476673cc270b125aa2d909212391d1a2001b35c8260a8772126b99dfd76b39faaa286feb7c43239185fe584bd4dc2bc04a64044ce

a357293 Use MakeUnique<Db>(...) (practicalswift) 3e09b39 Use MakeUnique<T>(...) instead of std::unique_ptr<T>(new T(...)) (practicalswift) 8617989 Add MakeUnique (substitute for C++14 std::make_unique) (practicalswift) d223bc9 Use unique_ptr for pcoinscatcher/pcoinsdbview/pcoinsTip/pblocktree (practicalswift) b45c597 Use unique_ptr for pdbCopy (Db) and fix potential memory leak (practicalswift) 29ab96d Use unique_ptr for dbenv (DbEnv) (practicalswift) f72cbf9 Use unique_ptr for pfilter (CBloomFilter) (practicalswift) 8ccf1bb Use unique_ptr for sem{Addnode,Outbound} (CSemaphore) (practicalswift) 73db063 Use unique_ptr for upnp_thread (boost::thread) (practicalswift) 0024531 Use unique_ptr for dbw (CDBWrapper) (practicalswift) fa6d122 Use unique_ptr:s for {fee,short,long}Stats (TxConfirmStats) (practicalswift) 5a6f768 Use unique_ptr for httpRPCTimerInterface (HTTPRPCTimerInterface) (practicalswift) 860e912 Use unique_ptr for pwalletMain (CWallet) (practicalswift) Pull request description: Use `std::unique_ptr` (C++11) where possible. Rationale: 1. Avoid resource leaks (specifically: forgetting to `delete` an object created using `new`) 2. Avoid undefined behaviour (specifically: double `delete`:s) **Note to reviewers:** Please let me know if I've missed any obvious `std::unique_ptr` candidates. Hopefully this PR should cover all the trivial cases. Tree-SHA512: 9fbeb47b800ab8ff4e0be9f2a22ab63c23d5c613a0c6716d9183db8d22ddbbce592fb8384a8b7874bf7375c8161efb13ca2197ad6f24b75967148037f0f7b20c

a8b5d20 Reset pblocktree before deleting LevelDB file (Sjors Provoost) Pull request description: bitcoin#11043 repaced: ``` delete pblocktree; pblocktree = new CBlockTreeDB(nBlockTreeDBCache, false, fReset); ``` With: ``` pblocktree.reset(new CBlockTreeDB(nBlockTreeDBCache, false, fReset)); ``` This is problematic because `new CBlockTreeDB` tries to delete the existing file, which will fail with `LOCK: already held by process` if it's still open. That's the case for QT. When QT finds a problem with the index it will ask the user if they want to reindex. At that point it has already opened `blocks/index`. It then runs this [while loop](https://github.com/bitcoin/bitcoin/blob/v0.16.0rc3/src/init.cpp#L1415) again with `fReset = 1`, resulting in the above error. This change makes that error go away, presumably because `reset()` without an argument closes the file. Tree-SHA512: fde8b546912f6773ac64da8476673cc270b125aa2d909212391d1a2001b35c8260a8772126b99dfd76b39faaa286feb7c43239185fe584bd4dc2bc04a64044ce

a357293 Use MakeUnique<Db>(...) (practicalswift) 3e09b39 Use MakeUnique<T>(...) instead of std::unique_ptr<T>(new T(...)) (practicalswift) 8617989 Add MakeUnique (substitute for C++14 std::make_unique) (practicalswift) d223bc9 Use unique_ptr for pcoinscatcher/pcoinsdbview/pcoinsTip/pblocktree (practicalswift) b45c597 Use unique_ptr for pdbCopy (Db) and fix potential memory leak (practicalswift) 29ab96d Use unique_ptr for dbenv (DbEnv) (practicalswift) f72cbf9 Use unique_ptr for pfilter (CBloomFilter) (practicalswift) 8ccf1bb Use unique_ptr for sem{Addnode,Outbound} (CSemaphore) (practicalswift) 73db063 Use unique_ptr for upnp_thread (boost::thread) (practicalswift) 0024531 Use unique_ptr for dbw (CDBWrapper) (practicalswift) fa6d122 Use unique_ptr:s for {fee,short,long}Stats (TxConfirmStats) (practicalswift) 5a6f768 Use unique_ptr for httpRPCTimerInterface (HTTPRPCTimerInterface) (practicalswift) 860e912 Use unique_ptr for pwalletMain (CWallet) (practicalswift) Pull request description: Use `std::unique_ptr` (C++11) where possible. Rationale: 1. Avoid resource leaks (specifically: forgetting to `delete` an object created using `new`) 2. Avoid undefined behaviour (specifically: double `delete`:s) **Note to reviewers:** Please let me know if I've missed any obvious `std::unique_ptr` candidates. Hopefully this PR should cover all the trivial cases. Tree-SHA512: 9fbeb47b800ab8ff4e0be9f2a22ab63c23d5c613a0c6716d9183db8d22ddbbce592fb8384a8b7874bf7375c8161efb13ca2197ad6f24b75967148037f0f7b20c

7a5d181 Use the character based overload for std::string::find. (Alin Rus) c19401b Don't use pass by reference to const for cheaply-copied types (bool, char, etc.). (practicalswift) 4c5fe36 [Refactor] Remove unused fQuit var from checkqueue.h (donaloconnor) fda7a5f Cleanup: remove unneeded header includes (random-zebra) ac2476c Add test cases covering the relevant key length boundaries: 64 bytes +/- 1 byte for HMAC-SHA256 and 128 bytes +/- 1 byte for HMAC-SHA512 (practicalswift) a346262 Fix code constness in CBlockIndex::GetAncestor() overloads (Dan Raviv) 65e3f4e Refactor: More range-based for loops (random-zebra) dd3d3c4 Use range-based for loops (C++11) when looping over map elements (practicalswift) 5a7750a Move RPC registration out of AppInitParameterInteraction (Russell Yanofsky) 402b4c4 Use compile-time constants instead of unnamed enumerations (practicalswift) bbac2d0 [Trivial] Fix indentation in coins.cpp (random-zebra) e539c62 Small refactor of CCoinsViewCache::BatchWrite() (Dan Raviv) ec91759 Use MakeUnique<T>(...) instead of std::unique_ptr<T>(new T(...)) (random-zebra) 93487b1 Use unique_ptr for pcoinscatcher/pcoinsdbview/pcoinsTip/pblocktree (random-zebra) ff43d69 Use unique_ptr for pdbCopy (Db) and fix potential memory leak (practicalswift) b4d9641 Use unique_ptr for dbenv (DbEnv) (practicalswift) 36108b9 Use unique_ptr for pfilter (CBloomFilter) (practicalswift) ff1c454 Use unique_ptr for sem{Addnode,Outbound} (CSemaphore) (practicalswift) 93daf17 Use unique_ptr for httpRPCTimerInterface (HTTPRPCTimerInterface) (practicalswift) 020ac16 Init: Remove redundant exit(EXIT_FAILURE) instances and replace with (random-zebra) b9f5d1f Remove duplicate uriParts.size() > 0 check (practicalswift) 440d961 Remove redundant check (!ecc is always true) (practicalswift) bfd295b Remove redundant NULL checks after new (practicalswift) 97aad32 Make fUseCrypto atomic (MeshCollider) 2711f78 Consistent parameter names in txdb.h (MeshCollider) d40df3a Fix race for mapBlockIndex in AppInitMain (random-zebra) 03b7766 Cleanup: remove unused functions to Hash the concat of 4 or more objects (random-zebra) c520e0f Remove some unused functions and methods (Pieter Wuille) 508f1a1 range-based loops and const qualifications in net.cpp (Marko Bencun) 79b1e50 Refactor: implement CPubKey::data() (random-zebra) 614d26c Refactor: more &vec[0] to vec.data() (random-zebra) 02b6337 Changing &vec[0] to vec.data(), what 9804 missed (MeshCollider) c1c8b05 Ensure that data types are consistent (jjz) 732bb9d Fix potential null dereferences (MeshCollider) 80f35f9 Remove unreachable code (practicalswift) Pull request description: This is a collection of simple refactorings coming from upstream Bitcoin v0.16 (adapting/extending to PIVX-specific code where needed). Pull requests backported: - bitcoin#10845 (practicalswift) - bitcoin#11238 (MeshCollider) - bitcoin#11232 (jjz) - bitcoin#10793 (MeshCollider) - bitcoin#10888 (benma) - ~~bitcoin#11351 (danra)~~ [edit: removed - included in #2423] - bitcoin#11385 (sipa) - bitcoin#11107 (MeshCollider) - bitcoin#10898 (practicalswift) - bitcoin#11511 (donaloconnor) - bitcoin#11043 (practicalswift) - bitcoin#11353 (danra) - bitcoin#10749 (practicalswift) - bitcoin#11603 (ryanofsky) - bitcoin#10493 (practicalswift) - bitcoin#11337 (danra) - bitcoin#11516 (practicalswift) - bitcoin#10574 (practicalswift) - bitcoin#12108 (donaloconnor) - bitcoin#10839 (practicalswift) - bitcoin#12159 (kekimusmaximus) ACKs for top commit: Fuzzbawls: ACK 7a5d181 furszy: re-ACK 7a5d181 after rebase, no code changes. Merging.. Tree-SHA512: d92f5df47f443391a6531274a2efb9a4882c62d32eff628f795b3abce703f108c8b40ec80ac841cde6c5fdd5c9ff2b6056a31546ac2edda279f5f18fccc99c32

a357293 Use MakeUnique<Db>(...) (practicalswift) 3e09b39 Use MakeUnique<T>(...) instead of std::unique_ptr<T>(new T(...)) (practicalswift) 8617989 Add MakeUnique (substitute for C++14 std::make_unique) (practicalswift) d223bc9 Use unique_ptr for pcoinscatcher/pcoinsdbview/pcoinsTip/pblocktree (practicalswift) b45c597 Use unique_ptr for pdbCopy (Db) and fix potential memory leak (practicalswift) 29ab96d Use unique_ptr for dbenv (DbEnv) (practicalswift) f72cbf9 Use unique_ptr for pfilter (CBloomFilter) (practicalswift) 8ccf1bb Use unique_ptr for sem{Addnode,Outbound} (CSemaphore) (practicalswift) 73db063 Use unique_ptr for upnp_thread (boost::thread) (practicalswift) 0024531 Use unique_ptr for dbw (CDBWrapper) (practicalswift) fa6d122 Use unique_ptr:s for {fee,short,long}Stats (TxConfirmStats) (practicalswift) 5a6f768 Use unique_ptr for httpRPCTimerInterface (HTTPRPCTimerInterface) (practicalswift) 860e912 Use unique_ptr for pwalletMain (CWallet) (practicalswift) Pull request description: Use `std::unique_ptr` (C++11) where possible. Rationale: 1. Avoid resource leaks (specifically: forgetting to `delete` an object created using `new`) 2. Avoid undefined behaviour (specifically: double `delete`:s) **Note to reviewers:** Please let me know if I've missed any obvious `std::unique_ptr` candidates. Hopefully this PR should cover all the trivial cases. Tree-SHA512: 9fbeb47b800ab8ff4e0be9f2a22ab63c23d5c613a0c6716d9183db8d22ddbbce592fb8384a8b7874bf7375c8161efb13ca2197ad6f24b75967148037f0f7b20c

a8b5d20 Reset pblocktree before deleting LevelDB file (Sjors Provoost) Pull request description: bitcoin#11043 repaced: ``` delete pblocktree; pblocktree = new CBlockTreeDB(nBlockTreeDBCache, false, fReset); ``` With: ``` pblocktree.reset(new CBlockTreeDB(nBlockTreeDBCache, false, fReset)); ``` This is problematic because `new CBlockTreeDB` tries to delete the existing file, which will fail with `LOCK: already held by process` if it's still open. That's the case for QT. When QT finds a problem with the index it will ask the user if they want to reindex. At that point it has already opened `blocks/index`. It then runs this [while loop](https://github.com/bitcoin/bitcoin/blob/v0.16.0rc3/src/init.cpp#L1415) again with `fReset = 1`, resulting in the above error. This change makes that error go away, presumably because `reset()` without an argument closes the file. Tree-SHA512: fde8b546912f6773ac64da8476673cc270b125aa2d909212391d1a2001b35c8260a8772126b99dfd76b39faaa286feb7c43239185fe584bd4dc2bc04a64044ce

practicalswift force-pushed the unique-pointers branch 5 times, most recently from 9a2ef33 to 8bdd63b Compare August 14, 2017 15:23

sipa reviewed Aug 14, 2017

View reviewed changes

practicalswift force-pushed the unique-pointers branch from 8bdd63b to 5a92b9d Compare August 14, 2017 20:21

ryanofsky reviewed Aug 14, 2017

View reviewed changes

practicalswift force-pushed the unique-pointers branch from 5a92b9d to 2f22831 Compare August 15, 2017 04:46

fanquake added the Refactoring label Aug 15, 2017

practicalswift force-pushed the unique-pointers branch from 1ecd701 to bb8dc2e Compare August 15, 2017 08:03

laanwj reviewed Aug 15, 2017

View reviewed changes

practicalswift force-pushed the unique-pointers branch from bb8dc2e to c152cca Compare August 15, 2017 08:07

laanwj reviewed Aug 15, 2017

View reviewed changes

practicalswift force-pushed the unique-pointers branch from c152cca to 3f87b86 Compare August 15, 2017 08:15

practicalswift force-pushed the unique-pointers branch from 3f87b86 to 6b78633 Compare August 16, 2017 03:22

ryanofsky reviewed Aug 16, 2017

View reviewed changes

practicalswift force-pushed the unique-pointers branch from 6b78633 to fac2368 Compare August 16, 2017 14:20

practicalswift force-pushed the unique-pointers branch from fac2368 to 1c2b9d6 Compare August 22, 2017 08:04

practicalswift added 8 commits November 9, 2017 16:52

Use unique_ptr for sem{Addnode,Outbound} (CSemaphore)

8ccf1bb

Use unique_ptr for pfilter (CBloomFilter)

f72cbf9

Use unique_ptr for dbenv (DbEnv)

29ab96d

Use unique_ptr for pdbCopy (Db) and fix potential memory leak

b45c597

Use unique_ptr for pcoinscatcher/pcoinsdbview/pcoinsTip/pblocktree

d223bc9

* pcoinscatcher (CCoinsViewErrorCatcher) * pcoinsdbview (CCoinsViewDB) * pcoinsTip (CCoinsViewCache) * pblocktree (CBlockTreeDB) * Remove variables shadowing pcoinsdbview

Add MakeUnique (substitute for C++14 std::make_unique)

8617989

From @ryanofsky:s bitcoin#10973. Thanks!

Use MakeUnique<T>(...) instead of std::unique_ptr<T>(new T(...))

3e09b39

Use MakeUnique<Db>(...)

a357293

practicalswift force-pushed the unique-pointers branch from 0a38fb9 to a357293 Compare November 9, 2017 15:54

practicalswift mentioned this pull request Nov 9, 2017

Remove redundant nullptr checks before deallocation #10696

Merged

laanwj merged commit a357293 into bitcoin:master Nov 9, 2017

Sjors mentioned this pull request Feb 9, 2018

Reset pblocktree before deleting LevelDB file #12401

Merged

practicalswift deleted the unique-pointers branch April 10, 2021 19:33

random-zebra mentioned this pull request Jun 29, 2021

[Refactoring] Bitcoin 0.16 cherries PIVX-Project/PIVX#2462

Merged

bitcoin locked as resolved and limited conversation to collaborators Aug 16, 2022

Use std::unique_ptr (C++11) where possible #11043

Use std::unique_ptr (C++11) where possible #11043

Uh oh!

Conversation

practicalswift commented Aug 14, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sipa Aug 14, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

practicalswift commented Aug 14, 2017

Uh oh!

ryanofsky left a comment

Choose a reason for hiding this comment

Uh oh!

ryanofsky left a comment

Choose a reason for hiding this comment

Uh oh!

ryanofsky Aug 14, 2017

Choose a reason for hiding this comment

Uh oh!

ryanofsky Aug 14, 2017

Choose a reason for hiding this comment

Uh oh!

laanwj commented Aug 15, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bytting commented Aug 15, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

practicalswift commented Aug 15, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

laanwj commented Aug 15, 2017

Uh oh!

laanwj Aug 15, 2017

Choose a reason for hiding this comment

Uh oh!

practicalswift Aug 15, 2017

Choose a reason for hiding this comment

Uh oh!

laanwj Aug 15, 2017

Choose a reason for hiding this comment

Uh oh!

practicalswift Aug 15, 2017

Choose a reason for hiding this comment

Uh oh!

laanwj commented Aug 15, 2017

Uh oh!

practicalswift commented Aug 15, 2017

Uh oh!

ryanofsky left a comment

Choose a reason for hiding this comment

Uh oh!

practicalswift commented Aug 16, 2017

Uh oh!

laanwj commented Aug 22, 2017

Uh oh!

practicalswift commented Aug 22, 2017

Uh oh!

practicalswift commented Nov 9, 2017

Uh oh!

laanwj commented Nov 9, 2017

Uh oh!

Uh oh!

practicalswift commented Aug 14, 2017 •

edited

Loading

sipa Aug 14, 2017 •

edited

Loading

laanwj commented Aug 15, 2017 •

edited

Loading

bytting commented Aug 15, 2017 •

edited

Loading

practicalswift commented Aug 15, 2017 •

edited

Loading