Skip to content

NameResolutionPal.Unix: fixed loss of address family information in async lookup #47171

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
wfurt merged 2 commits into from
Jan 22, 2021
Merged

NameResolutionPal.Unix: fixed loss of address family information in async lookup #47171

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
9127a5b
Removed dead code from SystemNative_GetHostEntryForNameAsync
gfoidl Jan 19, 2021
ff88f17
Fixed loss of address-family
gfoidl Jan 19, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 14 additions & 22 deletions src/libraries/Native/Unix/System.Native/pal_networking.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -601,12 +601,6 @@ int32_t SystemNative_GetHostEntryForNameAsync(const uint8_t* address, int32_t ad
return GetAddrInfoErrorFlags_EAI_BADARG;
}

sa_family_t platformFamily;
if (!TryConvertAddressFamilyPalToPlatform(addressFamily, &platformFamily))
{
return GetAddrInfoErrorFlags_EAI_FAMILY;
}

@stephentoub stephentoub Jan 21, 2021

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

What makes this dead?

@wfurt wfurt Jan 22, 2021

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

It seems like the TrySetAddressFamily() bellow does it inside so it seems like duplication to me.

@stephentoub stephentoub Jan 22, 2021

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Ah, not that it's dead code but that it's unnecessary because the same check is performed immediately after. Ok.

@gfoidl gfoidl Jan 22, 2021

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Now

runtime/src/libraries/Native/Unix/System.Native/pal_networking.c

Line 617 in 311de30

if (!TrySetAddressFamily(addressFamily, &state->hint))
sets the address family (after the code for sync and async got unified).

The removed code is a left-over from before that unification. The platformFamily isn't used anymore, so it's "dead".
This was forgotten in the other PR (due the work in single file host build).

(For completeness...late reply due being in different timezone 😉).


struct GetAddrInfoAsyncState* state = malloc(sizeof(*state) + addrlen + 1);

if (state == NULL)
Expand All @@ -622,24 +616,22 @@ int32_t SystemNative_GetHostEntryForNameAsync(const uint8_t* address, int32_t ad

memcpy(state->address, address, addrlen + 1);

*state = (struct GetAddrInfoAsyncState) {
.gai_request = {
.ar_name = state->address,
.ar_service = NULL,
.ar_request = &state->hint,
.ar_result = NULL
},
.gai_requests = &state->gai_request,
.sigevent = {
.sigev_notify = SIGEV_THREAD,
.sigev_value = {
.sival_ptr = state
},
.sigev_notify_function = GetHostEntryForNameAsyncComplete
state->gai_request = (struct gaicb) {
.ar_name = state->address,
.ar_service = NULL,
.ar_request = &state->hint,
.ar_result = NULL
};
state->gai_requests = &state->gai_request;
state->sigevent = (struct sigevent) {
.sigev_notify = SIGEV_THREAD,
.sigev_value = {
.sival_ptr = state
},
.entry = entry,
.callback = callback
.sigev_notify_function = GetHostEntryForNameAsyncComplete
};
state->entry = entry;
state->callback = callback;

atomic_thread_fence(memory_order_release);

Expand Down