search

PinkDev / Pink2

MIT License
16 stars 8 forks source link

gcc complains about this ; #12

Closed kirj closed 7 years ago

kirj commented 7 years ago

wallet.h:311: if (IsMine(txOut)); { nReward += GetCredit(txOut); if (!MoneyRange(nReward)) printf("CWallet::GetCredit() : value out of range"); }

PinkDev commented 7 years ago

What platform are you compiling on and what is the error?

kirj commented 7 years ago

Really? Linux, g++ 6.x "if (IsMine(txOut));" is a no-op, unless IsMine() throws something.

It's more like an entry to http://www.underhanded-c.org/ :-)

PinkDev commented 7 years ago

I misunderstood, I thought you were having trouble compiling - in which case knowing the platform you're attempting to do so on is relevant (and 'linux' couldn't be more generic and useless information given the broad range of distributions).

This is one of hundreds of nonsense pieces of crufty, albeit harmless, code inherited from the pedigree of codebases that Pink is based on. Yes I have a mess to clean up, and I'm very well aware of that.

If you suspect a security vulnerability in the code please do let me know. This particular piece of code, much like all the other garbage in the code, is essentially harmless and can be left until I have time to comb through the codebase and clean house better. IsMine() just checks to see if a transaction (in this case) belongs to the wallet calling the function. Obviously nothing else happens. Yes I realize that means that GetReward() itself does nothing as it stands, and looking at the code it looks like some lazy programmer couldn't figure out why this piece of the code wasn't working properly in the only function it gets called in, and wrote some code around it.

g++ will compile it just fine though, it just warns about this piece of code, along with tons of other garbage and poorly written code.

This will all get taken care of. I've combed through the warnings looking for anything concerning from a security standpoint, and I do appreciate more eyes looking for that. But stuff like this particular snip of code is on the back burner for the moment.

kirj commented 7 years ago

Thanks for explaining. It just looked like your wallet's reward calculation could go wrong. But there's another check to IsMine() in getCredit(const CTxOut& txout), as called by getReward(), which kind of fixes the missing if().