[libc-commits] [libc] [libc] Implement IPv6 (AF_INET6) support in inet_pton (PR #226550)

[Pavel Labath via libc-commits]

https://github.com/labath commented:

The patch seems technically correct, but it has a very brute force feel to it. I'd like to consider a couple of changes to make the code flow nicer. I'm not saying you have to apply all of these, but if you're not doing that, I'd like to hear the reason.

1. Using str_to_integer. I didn't push for that in the v4 version, as checking for leading zeroes looked messy, but here we just need to filter out leading spaces and "0x", which seems easy enough (we do it in `inet_addr`). And I'm hoping that doing this one component at a time will let us get rid of the `token_start` thingy and of the `if (num_digits > 0) {` block after the loop. I'm hoping for structure like:

```
while(!src.empty()) {
  if (src.starts_with("..")) ...;
  if (src[0] == ':') ...;
  if (internal::isspace(src[0]) || src.starts_with("0x") || src.starts_with("0X")) return 0;
  auto result = internal::strtointeger<uint16_t>(src.data(), 16, src.size());
  if (result.has_error() || result.parsed_len == 0 || result.parsed_len > 4)
    return 0;
  // extract number, advance by result.parsed_len
}
```

2. The logic for expanding the "::" contains a lot of scary array index computations. I wonder if we could clean that up by using separate vectors for the two parts. So we can do something like:

```
FixedVector<uint16_t> parts[2];
// parse
uint16_t *ptr = dst.s6_addr16;
inline_memcpy(ptr, parts[0].data(), parts[0].size()*sizeof(uint16_t));
ptr += parts[0].size();
size_t num_zeroes=16-parts[0].size()-parts[1].size()
inline_bzero(ptr, num_zeroes*sizeof(uint16_t));
ptr+=num_zeroes;
inline_memcpy(ptr, parts[1].data(), parts[1].size()*sizeof(uint16_t));
```

https://github.com/llvm/llvm-project/pull/226550