Details

I like it; have you thought of any negative tests? ie, testing that parsing invalid addresses (like feeding link_ntoa -> link_addr as you mentioned) fails?

In D50062#1141970, @adrian wrote:

I like it; have you thought of any negative tests? ie, testing that parsing invalid addresses (like feeding link_ntoa -> link_addr as you mentioned) fails?

so a fun thing about link_addr is it has no way to indicate an error, it will just return garbage. D49936 will fix that, but i wanted to get the tests in first so i can change the other diff a bit and make sure i don’t break it. i’ll add some negative tests there at the same time.

In D50062#1141980, @ivy wrote:

In D50062#1141970, @adrian wrote:

I like it; have you thought of any negative tests? ie, testing that parsing invalid addresses (like feeding link_ntoa -> link_addr as you mentioned) fails?

so a fun thing about link_addr is it has no way to indicate an error, it will just return garbage. D49936 will fix that, but i wanted to get the tests in first so i can change the other diff a bit and make sure i don’t break it. i’ll add some negative tests there at the same time.

aaahahahahahahahah :p

adrian accepted this revision as: adrian.Apr 29 2025, 1:58 PM

This revision is now accepted and ready to land.Apr 29 2025, 1:58 PM

ngie accepted this revision.May 3 2025, 3:17 PM

ngie added a subscriber: ngie.

ngie added inline comments.

lib/libc/tests/net/link_addr_test.cc
44	The following code seems like it’s a logical item to add to a common C/C++ library. I highly doubt that this logic hasn’t been written somewhere or relied upon in the past.
169	Some other potential testcases: positive: all caps addresses (dash, colon, and period delimited). negative: mixed delimiters input too short input too long NULL address (is crashing acceptable behavior?) invalid, e.g., non-hex, character set (zz:yy…). valid IPv6 addresses that seem like valid link addresses (shortened) IPv4 addresses

add some additional tests

address with mixed case hex digits
short address
long address
very long address (SA-16:37.libc)

This revision now requires review to proceed.May 4 2025, 5:51 AM

Harbormaster completed remote builds in B63863: Diff 154773.May 4 2025, 5:51 AM

ivy added inline comments.May 4 2025, 5:52 AM

lib/libc/tests/net/link_addr_test.cc
169	i've added a test for a mixed-case address; i wasn't sure if it was worth duplicating the other tests with different case. i've also added tests for short and long addresses, but they aren't negative tests since link_addr is supposed to accept them.