Skip to content

Conversation

@sipa
Copy link
Member

@sipa sipa commented Jul 7, 2017

And prefer a static_cast to the intended reference type.

Copy link
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
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.

Copy link
Contributor

@promag promag Jul 7, 2017

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
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
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
Copy link
Contributor

promag commented Jul 7, 2017

tACK.

Copy link
Contributor

@gmaxwell gmaxwell left a comment

Choose a reason for hiding this comment

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

utACK

@TheBlueMatt
Copy link
Contributor

utACK 0aadc11

@laanwj
Copy link
Member

laanwj commented Jul 10, 2017

And prefer a static_cast to the intended reference type.

What is the rationale here? Just code cleanup?

@sipa
Copy link
Member Author

sipa commented Jul 10, 2017

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

@jonasschnelli
Copy link
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
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