Skip to content

Fix getaddrinfo error reporting#278

Merged
Enmk merged 4 commits into
ClickHouse:masterfrom
nUl1:fix-gaierror
Jan 19, 2023
Merged

Fix getaddrinfo error reporting#278
Enmk merged 4 commits into
ClickHouse:masterfrom
nUl1:fix-gaierror

Conversation

@nUl1

@nUl1 nUl1 commented Jan 5, 2023

Copy link
Copy Markdown
Contributor

getaddrinfo error reporting is currently broken on non-Windows platforms as only a subset of its errors are reported via errno.
(On Windows everything gets combined in WSAGetLastError() and gai_strerror is declared not thread-safe for some reason.)

@CLAassistant

CLAassistant commented Jan 5, 2023

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

Comment thread clickhouse/base/socket.h

#if defined(_unix_)

class getaddrinfoErrorCategory : public std::error_category {

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.

Do you have to make it part of the API, i.e. visible to the users? IMO it would be better to move declaration to .cpp

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

There's no requirement for it to be public. But users will have access to it through e.code().category() and nothing to compare it to.

@Enmk Enmk merged commit 5353131 into ClickHouse:master Jan 19, 2023
@nUl1 nUl1 deleted the fix-gaierror branch February 13, 2023 11:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants