Skip to content

IoHandle: simplify interfaceName()#19448

Closed
rgs1 wants to merge 2 commits intoenvoyproxy:mainfrom
rgs1:interface-name-cleanup
Closed

IoHandle: simplify interfaceName()#19448
rgs1 wants to merge 2 commits intoenvoyproxy:mainfrom
rgs1:interface-name-cleanup

Conversation

@rgs1
Copy link
Copy Markdown
Member

@rgs1 rgs1 commented Jan 7, 2022

Was reading through #18531 and seeing the socket address being looked
up for each interface confused me a bit, so let's do it only once.

Signed-off-by: Raul Gutierrez Segales rgs@pinterest.com

Was reading through envoyproxy#18531 and seeing the socket address being looked
up for each interface confused me a bit, so let's do it only once.

Signed-off-by: Raul Gutierrez Segales <rgs@pinterest.com>
@rgs1
Copy link
Copy Markdown
Member Author

rgs1 commented Jan 7, 2022

cc: @junr03

Signed-off-by: Raul Gutierrez Segales <rgs@pinterest.com>
RELEASE_ASSERT(!rc.return_value_, fmt::format("getiffaddrs error: {}", rc.errno_));

absl::optional<std::string> selected_interface_name{};
absl::uint128 socket_address_value = socket_address->ip()->version() == Address::IpVersion::v4
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.

I think the prior version would return an empty interface name but I think falling through here might segfault instead. Maybe add a unit test?

@junr03
Copy link
Copy Markdown
Member

junr03 commented Feb 2, 2022

@rgs1 are you planning on updating with Alyssa's comment?

@alyssawilk
Copy link
Copy Markdown
Contributor

I think he's out on leave right now, so I assume it'll wait until he gets back

@github-actions
Copy link
Copy Markdown

github-actions bot commented Mar 4, 2022

This pull request has been automatically marked as stale because it has not had activity in the last 30 days. It will be closed in 7 days if no further activity occurs. Please feel free to give a status update now, ping for review, or re-open when it's ready. Thank you for your contributions!

@github-actions github-actions bot added the stale stalebot believes this issue/PR has not been touched recently label Mar 4, 2022
@github-actions
Copy link
Copy Markdown

This pull request has been automatically closed because it has not had activity in the last 37 days. Please feel free to give a status update now, ping for review, or re-open when it's ready. Thank you for your contributions!

@github-actions github-actions bot closed this Mar 11, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

stale stalebot believes this issue/PR has not been touched recently waiting

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants