Skip to content
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

Jump to bottom

NameResolutionPal.Unix enabled async name resolution #34633

Merged
merged 37 commits into from
Jan 14, 2021
Merged

NameResolutionPal.Unix enabled async name resolution #34633

Changes from 1 commit
Commits
Show all changes
37 commits
Select commit Hold shift + click to select a range
fc09ca1
Native implementation
gfoidl Apr 6, 2020
8c955bc
Native tests
gfoidl Apr 6, 2020
cf13c70
Managed implementation
gfoidl Apr 6, 2020
4614267
Managed tests
gfoidl Apr 6, 2020
1b2daab
Revert Interop.GetHostName change -- it's needed at other places too
gfoidl Apr 7, 2020
b9629e9
Fixed builds failures due to "unused parameter"
gfoidl Apr 7, 2020
6f64c86
Native: move struct addrinfo hint from stack-alloc to heap-alloc
gfoidl Apr 7, 2020
e7df0cc
PR feedback
gfoidl Apr 8, 2020
d7cb2be
Fixed leak in tests.c according to valgrind's run
gfoidl Apr 9, 2020
74b3031
Fixed bug due to marshalled string argument
gfoidl Apr 9, 2020
775ff72
More managed PalTests
gfoidl Apr 9, 2020
ec8522f
Revert "Native tests"
gfoidl Apr 9, 2020
298cf03
Handle the case of HostName="" and tests for this
gfoidl Apr 9, 2020
458579b
Use flexible array member to hold the address in struct state
gfoidl Apr 9, 2020
8f69164
Nits in native layer
gfoidl Apr 10, 2020
8cec5fb
Merge branch 'master' into unix-async-name-resolution
gfoidl Jul 1, 2020
f3ebd6a
Merge branch 'master' into unix-async-name-resolution
gfoidl Aug 13, 2020
cf45cc9
Merge branch 'master' into unix-async-name-resolution
gfoidl Dec 15, 2020
43f889f
Fixed native merge conflict + added AddressFamily-handling
gfoidl Dec 15, 2020
bd16c74
Fixed managed merge conflicts + added AddressFamily-handling
gfoidl Dec 15, 2020
ed5c39f
Updated NameResolutionPalTests for AddressFamily
gfoidl Dec 15, 2020
b8efde5
Removed EnsureSocketsAreInitialized
gfoidl Dec 15, 2020
04dbd8e
Fixed bug at native side
gfoidl Dec 15, 2020
37e4dd3
Refactor setting of the address family into TrySetAddressFamily
gfoidl Dec 15, 2020
7176378
Fixed unused parameter warning / failure
gfoidl Dec 15, 2020
b64867b
Little cleanup
gfoidl Dec 16, 2020
900834e
Fixed (unrelated) test failures
gfoidl Dec 16, 2020
198717e
Made LoggingTest async instead of GetAwaiter().GetResult()
gfoidl Dec 16, 2020
f85c316
Use function pointer
gfoidl Dec 16, 2020
438c75a
Use OperatinngSystem instead of RuntimeInformation in tests
gfoidl Dec 18, 2020
89c266f
Merge branch 'master' into unix-async-name-resolution
gfoidl Dec 18, 2020
33da8da
PR Feedback for CMake
gfoidl Dec 18, 2020
fa9c6f9
Update src/libraries/Native/Unix/System.Native/extra_libs.cmake
VSadov Dec 18, 2020
8266dd3
Revert "Update src/libraries/Native/Unix/System.Native/extra_libs.cmake"
gfoidl Jan 8, 2021
75e52c7
Another to build single file host
gfoidl Jan 8, 2021
89a7c5a
Merge branch 'master' into unix-async-name-resolution
gfoidl Jan 14, 2021
dd1e245
Test for !Windows instead excluding several Unix-flavors
gfoidl Jan 14, 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
Prev Previous commit
Next Next commit
Fixed bug due to marshalled string argument
  • Loading branch information
@gfoidl
gfoidl committed Apr 9, 2020
commit 74b30319792fc5de02e61b1b3ea502672c1211d2
23 changes: 17 additions & 6 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 @@ -428,7 +428,7 @@ struct GetAddrInfoAsyncState
struct gaicb* gai_requests;
struct sigevent sigevent;

const uint8_t* address;
uint8_t* address;
HostEntry* entry;
GetHostEntryForNameCallback callback;
};
Expand Down Expand Up @@ -456,6 +456,7 @@ static void GetHostEntryForNameAsyncComplete(sigval_t context)
callback(state->entry, ret);
}

free(state->address);
free(state);
}
#endif
Expand Down Expand Up @@ -501,9 +502,11 @@ int32_t SystemNative_GetHostEntryForNameAsync(const uint8_t* address, HostEntry*
return GetAddrInfoErrorFlags_EAI_MEMORY;
}

*state = (struct GetAddrInfoAsyncState){
char* localAddress = strdup((const char*)address);
Copy link
Contributor

Choose a reason for hiding this comment

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

malloc() result is NULL-checked but strdup() isn't

Copy link
Contributor

@lpereira lpereira Apr 9, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

You could do both allocations here in a single one by using a flexible member in struct GetAddrInfoAsyncState:

struct GetAddrInfoAsyncState {
  ...
  char address[];
};

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

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

Also, I'd bail out if addrlen > 253 (hostnames can't be larger than that per the RFC; otherwise, you'd have to check for overflow before passing the len to malloc).


*state = (struct GetAddrInfoAsyncState) {
.gai_request = {
.ar_name = (const char*)address,
.ar_name = localAddress,
.ar_service = NULL,
.ar_request = &s_hostEntryForNameHints,
.ar_result = NULL
Expand All @@ -516,7 +519,7 @@ int32_t SystemNative_GetHostEntryForNameAsync(const uint8_t* address, HostEntry*
},
.sigev_notify_function = GetHostEntryForNameAsyncComplete
},
.address = address,
.address = (uint8_t*)localAddress,
.entry = entry,
.callback = callback
};
Expand All @@ -527,6 +530,7 @@ int32_t SystemNative_GetHostEntryForNameAsync(const uint8_t* address, HostEntry*

if (result != 0)
{
free(localAddress);
free(state);
return ConvertGetAddrInfoAndGetNameInfoErrorsToPal(result);
}
Expand All @@ -546,8 +550,15 @@ void SystemNative_FreeHostEntry(HostEntry* entry)
{
if (entry != NULL)
{
free(entry->CanonicalName);
free(entry->IPAddressList);
if (entry->CanonicalName != NULL)
{
free(entry->CanonicalName);
}

if (entry->IPAddressList != NULL)
{
free(entry->IPAddressList);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: You don't need to null-check before calling free(). free() can take any value returned by malloc(), including NULL. (Change is harmless, but something to keep in mind.)

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks for the valuable input!

Maybe it was an artefact of the bug hunt, but Valgrind complained this. That's why I changed this. Will revert to the cleaner version.


entry->CanonicalName = NULL;
entry->IPAddressList = NULL;
Expand Down