ConnectBlock(): fix error() format to be unsigned #2022

Merged
merged 1 commit into from Nov 17, 2012

Conversation

Projects
None yet
5 participants

Diapolo commented Nov 16, 2012

  • I introduced the wrong format macro with my former patch (#2018), this
    needs to be signed not unsigned (thanks Luke-Jr)

Diapolo commented Nov 16, 2012

These were the warning my patch fixes:
src\main.cpp: In member function 'bool CBlock::ConnectBlock(CBlockIndex*, CCoinsViewCache&, bool)':
src\main.cpp⚠️ unknown conversion type character 'l' in format [-Wformat]
src\main.cpp⚠️ unknown conversion type character 'l' in format [-Wformat]
src\main.cpp⚠️ too many arguments for format [-Wformat-extra-args]

Member

luke-jr commented Nov 16, 2012

Looks like a bug in Microsoft printf family of functions :(

ACK for 0.7.2 & master

Diapolo commented Nov 16, 2012

Thanks for taking a look and I wanted to clarify why I patched it :).

Owner

sipa commented Nov 16, 2012

ACK

Contributor

jgarzik commented Nov 17, 2012

Please fix the commit message. The one-line summary of your change, as it appears in "git shortlog" and other tools, is wholly useless: "fix pull #2018"

That is utterly meaningless without further research; it tells nothing about what code is modified, or how it was modified, or why it needed modification.

We use "git shortlog" to generate summaries for release notes and other purposes. The first line is a summary of the change, and for this, probably should be something like "ConnectBlock: fix error() format to be unsigned" etc.

Owner

laanwj commented Nov 17, 2012

How is this a bug in the Microsoft printf function @luke-jr?

Member

luke-jr commented Nov 17, 2012

@laanwj %lld is standard C printf format, but not supported by Microsoft's printf: http://sourceforge.net/tracker/?func=detail&atid=102435&aid=1963136&group_id=2435

Owner

laanwj commented Nov 17, 2012

Ooh, okay something made by MS is incompatible to the standards, now that's a now one, let's call the press :)

Owner

laanwj commented Nov 17, 2012

I still don't get those warnings though.
We define:

#ifndef PRI64d
#if defined(_MSC_VER) || defined(__MSVCRT__)
#define PRI64d  "I64d"
#define PRI64u  "I64u"
#define PRI64x  "I64x"
#else
#define PRI64d  "lld"
#define PRI64u  "llu"
#define PRI64x  "llx"
#endif
#endif

Which means that in the case of MS, we use "special" retarded character sequences. The thing is, there is no 'l' in any of those. So where does that warning come from?

Member

luke-jr commented Nov 17, 2012

@laanwj The original code which Diapolo is referring to used %lld; he replaced that with PRI64u, but it needs to be PRI64d since the argument is signed.

Owner

laanwj commented Nov 17, 2012

Right, I get it now, thanks

Philip Kaufmann
ConnectBlock(): fix error() format to be unsigned
- I introduced the wrong format macro with my former patch (#2018), this
  needs to be signed not unsigned (thanks Luke-Jr)

Diapolo commented Nov 17, 2012

@jgarzik Sorry, yes it was a weak commit message :), fixed it!

laanwj added a commit that referenced this pull request Nov 17, 2012

@laanwj laanwj merged commit 4725e96 into bitcoin:master Nov 17, 2012

laudney pushed a commit to reddcoin-project/reddcoin that referenced this pull request Mar 19, 2014

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