Skip to content

Avoid dereference-of-casted-pointer#10760

Merged
sipa merged 1 commit into
bitcoin:masterfrom
sipa:20170707_avoidcastptr
Jul 15, 2017
Merged

Avoid dereference-of-casted-pointer#10760
sipa merged 1 commit into
bitcoin:masterfrom
sipa:20170707_avoidcastptr

Conversation

@sipa

@sipa sipa commented Jul 7, 2017

Copy link
Copy Markdown
Member

And prefer a static_cast to the intended reference type.

Comment thread src/core_memusage.h Outdated

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Remove static cast?

  return memusage::DynamicUsage(script);

In my system it builds fine.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Nice catch, fixed.

Comment thread src/wallet/walletdb.cpp Outdated

@promag promag Jul 7, 2017

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Removing the cast, shouldn't the compiler resolve to:

template<typename Stream, unsigned int N, typename T>
inline void Serialize(Stream& os, const prevector<N, T>& v)

If not I think we could implement CScript::Serialize to call super and drop all of these casts.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I somehow remembered that this didn't work, but it seems I was wrong. Fixed.

@practicalswift

Copy link
Copy Markdown
Contributor

Concept ACK 8be4385d64c5a50c63834b75c06d1b27d66ddb62

Regarding the static_cast preference, see also PR #10498: "Use static_cast instead of C-style casts for non-fundamental types".

@sipa
sipa force-pushed the 20170707_avoidcastptr branch from 8be4385 to 0aadc11 Compare July 7, 2017 17:46
@promag

promag commented Jul 7, 2017

Copy link
Copy Markdown
Contributor

tACK.

@gmaxwell gmaxwell left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

utACK

@TheBlueMatt

Copy link
Copy Markdown
Contributor

utACK 0aadc11

@laanwj

laanwj commented Jul 10, 2017

Copy link
Copy Markdown
Member

And prefer a static_cast to the intended reference type.

What is the rationale here? Just code cleanup?

@sipa

sipa commented Jul 10, 2017

Copy link
Copy Markdown
Member Author

Yes, cleanup. The existing practice risks hiding some bugs.

@jonasschnelli

Copy link
Copy Markdown
Contributor

utACK 0aadc11

@sipa
sipa merged commit 0aadc11 into bitcoin:master Jul 15, 2017
sipa added a commit that referenced this pull request Jul 15, 2017
0aadc11 Avoid dereference-of-casted-pointer (Pieter Wuille)

Pull request description:

  And prefer a static_cast to the intended reference type.

Tree-SHA512: e83b20023a4dca6029b46f7040a8a6fd54e1b42112ec0c87c3c3b567ed641de97a9e2335b57a2efb075491f641e5b977bc226a474276bea0c3c3c71d8d6ac54d
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Jul 24, 2019
0aadc11 Avoid dereference-of-casted-pointer (Pieter Wuille)

Pull request description:

  And prefer a static_cast to the intended reference type.

Tree-SHA512: e83b20023a4dca6029b46f7040a8a6fd54e1b42112ec0c87c3c3b567ed641de97a9e2335b57a2efb075491f641e5b977bc226a474276bea0c3c3c71d8d6ac54d
PastaPastaPasta added a commit to PastaPastaPasta/dash that referenced this pull request Jul 24, 2019
Signed-off-by: Pasta <pasta@dashboost.org>
furszy added a commit to PIVX-Project/PIVX that referenced this pull request Feb 6, 2021
206a6d6 Avoid dereference-of-casted-pointer (Pieter Wuille)

Pull request description:

  Straightforward cleanup, from bitcoin#10760

ACKs for top commit:
  furszy:
    re-utACK 206a6d6
  Fuzzbawls:
    utACK 206a6d6

Tree-SHA512: ef36994afe9fe2fc3b9ce5f88cdc77c89617bfd642d98a3fc83cb030063cc2ebb3537dca98b23e02a341a7e64d78fd5d715ee74a72b79c6ac455ecd67e841096
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Sep 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

9 participants