@jnewbery
(Attaching to an edit to make this a thread instead of top-level comments)
Good point about always evaluating the LogPrint args. In practice, I don’t think it matters very much here.
Yeah, I kind-of agree, just didn’t feel comfortable about it. I suspect:
0 pnode->Disconnect(strprintf("socket send error: %s", NetworkErrorString(nErr));
and
0void CNode::Disconnect(std::string msg)
1{
2 LogPrint(BCLog::NET, "%s; disconnecting peer=%d\n", msg, GetId());
3 fDisconnect = true;
4}
would work pretty well.
Could then retain the “only evaluate args when needed” part by adding a:
0#define LOGSTRPRINTF(category, fmt, ...) \
1 (LogAcceptCategory((category)) ? strprintf(fmt, __VA_ARGS__) : "")
to logging.h and making it be pnode->Disconnect(LOGSTRPRINTF(BCLog::NET, "socket send error: %s", NetworkErrorString(nErr)); but, as you say, not sure that’s actually worth the effort.
(Could potentially have Disconnect() check IsManualConn() and always log disconnection of manual connections too – though would need to do something cleverer to match this with the LOGSTRPRINTF idea…)
