Add a test for net::internal::Getifaddrs() [chromium/src : main]

0 views

Skip to first unread message

Adam Rice (Gerrit)

unread,

Dec 19, 2025, 8:21:45 AM (2 days ago) Dec 19

to Chromium LUCI CQ, chromium...@chromium.org, bnc+...@chromium.org, net-r...@chromium.org

Attention needed from mmenke

New activity on the change

Open in Gerrit

Related details

Attention is currently required from:

mmenke

Submit Requirements:

Code-Coverage
Code-Owners
Code-Review
Review-Enforcement

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

mmenke (Gerrit)

unread,

Dec 19, 2025, 10:58:46 AM (2 days ago) Dec 19

to Adam Rice, Chromium LUCI CQ, chromium...@chromium.org, bnc+...@chromium.org, net-r...@chromium.org

Attention needed from Adam Rice

mmenke voted and added 3 comments

Votes added by mmenke

Code-Review

3 comments

Patchset-level comments

File-level comment, Patchset 4 (Latest):

mmenke . resolved

LGTM!

It is wild that we have this workaround in Chrome rather than in Android. Workarounds for hardware/driver bugs in every app just doesn't scale. :(

File net/base/network_interfaces_getifaddrs_unittest.cc

Line 228, Patchset 4 (Latest):

  bool expect_0_byte = false;
  for (uint8_t byte : bytes) {

mmenke . unresolved

optional: This seems more complicated than necessary. We can only have the second block:

```
  bool seen_0_bit = false;
  for (uint8_t byte : bytes) {
    for (uint8_t bit = 128u; bit != 0; bit >>= 1) {
      if (seen_0_bit && (byte & bit) != 0) {
        return false;
      }
      if ((byte & bit) == 0) {
        seen_0_bit = true;
      }
    }
  }
```

I think implicitly separating out the ==0, ==0xFF, and other cases makes things strictly more complicated, and as this is single-use test-only code, perf doesn't matter.

Line 294, Patchset 4 (Latest): CheckBytesExist(base::as_bytes(base::span_from_ref(as_in->sin_addr)));

mmenke . unresolved

optional: Should we call this on the mask, too? That protects us against two things: Under bytes that IPEndPoint::FromSocketAddr() doesn't read being left unset (when run with memory sanitizers), and anything after the address not being allocated (which I guess may only be sin6_scope_id)