-
Notifications
You must be signed in to change notification settings - Fork 14.9k
[libc] implement inet_aton
#162651
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.
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
base: main
Are you sure you want to change the base?
[libc] implement inet_aton
#162651
Changes from all commits
9774e2d
67fbf2e
247a96e
c86be34
a8b5f06
ec35143
e2b5b52
87d19e8
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,57 @@ | ||||||||||||||||||||||||||||||||
//===-- Implementation of inet_aton function ------------------------------===// | ||||||||||||||||||||||||||||||||
// | ||||||||||||||||||||||||||||||||
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. | ||||||||||||||||||||||||||||||||
// See https://llvm.org/LICENSE.txt for license information. | ||||||||||||||||||||||||||||||||
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception | ||||||||||||||||||||||||||||||||
// | ||||||||||||||||||||||||||||||||
//===----------------------------------------------------------------------===// | ||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||
#include "src/arpa/inet/inet_aton.h" | ||||||||||||||||||||||||||||||||
#include "src/__support/common.h" | ||||||||||||||||||||||||||||||||
#include "src/__support/endian_internal.h" | ||||||||||||||||||||||||||||||||
#include "src/__support/str_to_integer.h" | ||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||
namespace LIBC_NAMESPACE_DECL { | ||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||
LLVM_LIBC_FUNCTION(int, inet_aton, (const char *cp, in_addr *inp)) { | ||||||||||||||||||||||||||||||||
constexpr int IPV4_MAX_DOT_NUM = 3; | ||||||||||||||||||||||||||||||||
unsigned long parts[IPV4_MAX_DOT_NUM + 1] = {0}; | ||||||||||||||||||||||||||||||||
int dot_num = 0; | ||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||
for (; dot_num <= IPV4_MAX_DOT_NUM; ++dot_num) { | ||||||||||||||||||||||||||||||||
auto result = internal::strtointeger<unsigned long>(cp, 0); | ||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this is a good use of this function, but you should add note that this may allow binary integers (with a leading There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I haven't tested it yet, but a quick look at the code suggests llvm-project/libc/src/__support/str_to_integer.h Lines 57 to 71 in 88ba06d
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Test case for binary integers added in c86be34. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ah, in that case I forgot we haven't added that yet. I'd say don't worry about testing that it doesn't work because it's not a problem if it does and we'll have to change it when binary support is added to strtointeger. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Yes, so I think that's why we want to keep the binary integers test cases. That way, once binary support is added, we can immediately know if anything needs to be handled on the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What would we need to add beyond a comment? Calling this function with an There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Binary integers test removed, undefined_behavior.rst added(Not quite sure about the wording). |
||||||||||||||||||||||||||||||||
parts[dot_num] = result; | ||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||
if (result.has_error() || result.parsed_len == 0) | ||||||||||||||||||||||||||||||||
return 0; | ||||||||||||||||||||||||||||||||
char next_char = *(cp + result.parsed_len); | ||||||||||||||||||||||||||||||||
if (next_char != '.' && next_char != '\0') | ||||||||||||||||||||||||||||||||
return 0; | ||||||||||||||||||||||||||||||||
else if (next_char == '\0') | ||||||||||||||||||||||||||||||||
break; | ||||||||||||||||||||||||||||||||
else | ||||||||||||||||||||||||||||||||
cp += (result.parsed_len + 1); | ||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||
if (dot_num > IPV4_MAX_DOT_NUM) | ||||||||||||||||||||||||||||||||
return 0; | ||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||
// converts the Internet host address cp from the IPv4 numbers-and-dots | ||||||||||||||||||||||||||||||||
// notation into binary form (in network byte order) | ||||||||||||||||||||||||||||||||
unsigned long result = 0; | ||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. for readability it would be good to have a comment explaining what the format you're parsing is. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Would you prefer the simple term or the comprehensive term?
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'd go with something shorter than a quote from the standard, but a bit more explanation than just There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Comment about the format added. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Looks good, I'd say add |
||||||||||||||||||||||||||||||||
for (int i = 0; i <= dot_num; ++i) { | ||||||||||||||||||||||||||||||||
unsigned long max_part = | ||||||||||||||||||||||||||||||||
i == dot_num ? (0xffffffffUL >> (8 * dot_num)) : 0xffUL; | ||||||||||||||||||||||||||||||||
if (parts[i] > max_part) | ||||||||||||||||||||||||||||||||
return 0; | ||||||||||||||||||||||||||||||||
int shift = i == dot_num ? 0 : 8 * (IPV4_MAX_DOT_NUM - i); | ||||||||||||||||||||||||||||||||
result |= parts[i] << shift; | ||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||
if (inp) | ||||||||||||||||||||||||||||||||
inp->s_addr = Endian::to_big_endian(static_cast<uint32_t>(result)); | ||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||
return 1; | ||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||
} // namespace LIBC_NAMESPACE_DECL |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,21 @@ | ||
//===-- Implementation header of inet_aton ----------------------*- C++ -*-===// | ||
// | ||
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. | ||
// See https://llvm.org/LICENSE.txt for license information. | ||
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception | ||
// | ||
//===----------------------------------------------------------------------===// | ||
|
||
#ifndef LLVM_LIBC_SRC_ARPA_INET_INET_ATON_H | ||
#define LLVM_LIBC_SRC_ARPA_INET_INET_ATON_H | ||
|
||
#include "include/llvm-libc-types/in_addr.h" | ||
#include "src/__support/macros/config.h" | ||
|
||
namespace LIBC_NAMESPACE_DECL { | ||
|
||
int inet_aton(const char *cp, in_addr *inp); | ||
|
||
} // namespace LIBC_NAMESPACE_DECL | ||
|
||
#endif // LLVM_LIBC_SRC_ARPA_INET_INET_ATON_H |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,92 @@ | ||
//===-- Unittests for inet_aton -------------------------------------------===// | ||
// | ||
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. | ||
// See https://llvm.org/LICENSE.txt for license information. | ||
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception | ||
// | ||
//===----------------------------------------------------------------------===// | ||
|
||
#include "src/arpa/inet/htonl.h" | ||
#include "src/arpa/inet/inet_aton.h" | ||
#include "test/UnitTest/Test.h" | ||
|
||
namespace LIBC_NAMESPACE_DECL { | ||
|
||
TEST(LlvmLibcInetAton, ValidTest) { | ||
in_addr a; | ||
|
||
// a.b.c.d | ||
a.s_addr = 0; | ||
ASSERT_EQ(1, inet_aton("127.1.2.4", &a)); | ||
ASSERT_EQ(htonl(0x7f010204), a.s_addr); | ||
|
||
// a.b.c | ||
a.s_addr = 0; | ||
ASSERT_EQ(1, inet_aton("127.1.4", &a)); | ||
ASSERT_EQ(htonl(0x7f010004), a.s_addr); | ||
|
||
// a.b | ||
a.s_addr = 0; | ||
ASSERT_EQ(1, inet_aton("127.1", &a)); | ||
ASSERT_EQ(htonl(0x7f000001), a.s_addr); | ||
|
||
// a | ||
a.s_addr = 0; | ||
ASSERT_EQ(1, inet_aton("0x7f000001", &a)); | ||
ASSERT_EQ(htonl(0x7f000001), a.s_addr); | ||
|
||
// Hex (0x) and mixed-case hex digits. | ||
a.s_addr = 0; | ||
ASSERT_EQ(1, inet_aton("0xFf.0.0.1", &a)); | ||
ASSERT_EQ(htonl(0xff000001), a.s_addr); | ||
|
||
// Hex (0X) and mixed-case hex digits. | ||
a.s_addr = 0; | ||
ASSERT_EQ(1, inet_aton("0XfF.0.0.1", &a)); | ||
ASSERT_EQ(htonl(0xff000001), a.s_addr); | ||
|
||
// Octal. | ||
a.s_addr = 0; | ||
ASSERT_EQ(1, inet_aton("0177.0.0.1", &a)); | ||
ASSERT_EQ(htonl(0x7f000001), a.s_addr); | ||
|
||
a.s_addr = 0; | ||
ASSERT_EQ(1, inet_aton("036", &a)); | ||
ASSERT_EQ(htonl(036U), a.s_addr); | ||
} | ||
|
||
TEST(LlvmLibcInetAton, InvalidTest) { | ||
ASSERT_EQ(0, inet_aton("", nullptr)); // Empty. | ||
ASSERT_EQ(0, inet_aton("x", nullptr)); // Leading junk. | ||
ASSERT_EQ(0, inet_aton("127.0.0.1x", nullptr)); // Trailing junk. | ||
ASSERT_EQ(0, inet_aton("09.0.0.1", nullptr)); // Invalid octal. | ||
ASSERT_EQ(0, inet_aton("0xg.0.0.1", nullptr)); // Invalid hex. | ||
ASSERT_EQ(0, inet_aton("1.2.3.4.5", nullptr)); // Too many dots. | ||
ASSERT_EQ(0, inet_aton("1.2.3.4.", nullptr)); // Trailing dot. | ||
|
||
// Out of range a.b.c.d form. | ||
ASSERT_EQ(0, inet_aton("999.0.0.1", nullptr)); | ||
ASSERT_EQ(0, inet_aton("0.999.0.1", nullptr)); | ||
ASSERT_EQ(0, inet_aton("0.0.999.1", nullptr)); | ||
ASSERT_EQ(0, inet_aton("0.0.0.999", nullptr)); | ||
|
||
// Out of range a.b.c form. | ||
ASSERT_EQ(0, inet_aton("256.0.0", nullptr)); | ||
ASSERT_EQ(0, inet_aton("0.256.0", nullptr)); | ||
ASSERT_EQ(0, inet_aton("0.0.0x10000", nullptr)); | ||
|
||
// Out of range a.b form. | ||
ASSERT_EQ(0, inet_aton("256.0", nullptr)); | ||
ASSERT_EQ(0, inet_aton("0.0x1000000", nullptr)); | ||
|
||
// Out of range a form. | ||
ASSERT_EQ(0, inet_aton("0x100000000", nullptr)); | ||
|
||
// 64-bit overflow. | ||
ASSERT_EQ(0, inet_aton("0x10000000000000000", nullptr)); | ||
|
||
// Out of range octal. | ||
ASSERT_EQ(0, inet_aton("0400.0.0.1", nullptr)); | ||
} | ||
|
||
} // namespace LIBC_NAMESPACE_DECL |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: replace
internal::strtointeger
with "the same code as strtol"