@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:
pnode->Disconnect(strprintf("socket send error: %s", NetworkErrorString(nErr));
and
void CNode::Disconnect(std::string msg)
{
LogPrint(BCLog::NET, "%s; disconnecting peer=%d\n", msg, GetId());
fDisconnect = true;
}
would work pretty well.
Could then retain the "only evaluate args when needed" part by adding a:
#define LOGSTRPRINTF(category, fmt, ...) \
(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...)
