Additional BSD & select improvements

[mzgrebnak]

nx_bsd_sendto
Aligning with POSIX. Const parameter before struct nx_bsd_sockaddr *destAddr.

nx_bsd_soc_close
Unlisten in case of failure in accepting connection due to resource limitation.

nx_bsd_inet_aton_impl
Aligning with POSIX. Use strict parsing to allow notation only with 4 numbers and 3 dots for ipv4.

nx_bsd_select
Aligning with POSIX. Optimizing writefds.
Fixing exceptfds.

nx_bsd_select_wakeup
Fixing exceptfds.

nx_bsd_inet_pton
Aligning with POSIX. Use strict parsing to allow notation only with 4 numbers and 3 dots for ipv4.

[fdesbiens]

Thanks for this — there are four independent improvements bundled here, and three of them are genuinely good catches. Excluding a listening socket from writefds, excluding a bound-but-unconnected TCP socket from writefds, and the unlisten on the failed-accept path are all real POSIX/resource bugs, and the added regression tests are welcome.

Two things need to change before this can land, and I'd like to discuss a third:

1. It breaks the v6_bsd_raw_build configuration. The const added to nx_bsd_sendto is not propagated to the two internal helpers it forwards to, and the library is compiled with -Werror. See the comment on nxd_bsd.c:3690 — this is a one-line-per-site fix.

2. Making inet_aton() strict is the wrong direction. inet_aton() is specified by 4.3BSD and implemented by glibc/musl/BSD to accept a, a.b, a.b.c and a.b.c.d; inet_pton() is the strict one. The PR makes inet_aton reject the abbreviated forms while leaving inet_addr lenient, which is also self-inconsistent, and it rewrites three previously-passing tests to expect the new failures. Strictness should be applied only at the inet_pton call sites. Details on nxd_bsd.c:6012.

Related: the rename to nx_bsd_inet_aton_impl removes nx_bsd_inet_aton from the public API, which breaks applications built with NX_BSD_ENABLE_NATIVE_API (nxd_bsd.h:1007), and the new function-like macro inverts the direction of the entire override block (nxd_bsd.h:236).

3. The nx_bsd_select_wakeup change looks redundant — NetX already delivers a URG notification through nx_bsd_tcp_exception_notify. See nxd_bsd.c:10334; happy to be corrected if you have a case in mind that the existing callback misses.

There are also a couple of correctness issues in the new select() exceptfds scan
(nxd_bsd.c:8388) and in the unlisten guard (nxd_bsd.c:5011), plus one test assertion that passes by coincidence (netx_bsd_pton_test.c:125).

Two project-policy items: the new use_strict_parsing parameter needs to be documented in the function's comment block , and the nx_bsd_sendto signature change needs a matching documentation update in rtos-docs-asciidoc.

Finally, a general style note: the new code uses if( and a->b, whereas this file consistently uses if ( and a -> b. Please match the surrounding style.

[fdesbiens]

Adding const here is correct and matches POSIX, but it isn't propagated to the two internal
helpers that sendto forwards destAddr to, so this breaks the build.

Both helpers still take a non-const pointer:

• declarations at nxd_bsd.c:178 and nxd_bsd.c:184
• definitions at nxd_bsd.c:12760 and nxd_bsd.c:13064
• call sites at nxd_bsd.c:3762 (NX_BSD_RAW_PPPOE_SUPPORT) and nxd_bsd.c:3765
  (NX_BSD_RAW_SUPPORT)

Compiling nxd_bsd.c with the CI flags and -DNX_BSD_RAW_SUPPORT:

PR head : nxd_bsd.c:3765:101: warning: passing argument 5 of
          ‘_nx_bsd_hardware_internal_sendto’ discards ‘const’ qualifier from
          pointer target type [-Wdiscarded-qualifiers]
dev     : clean

The library target is built with PRIVATE -Werror -Wall -Wextra
(test/cmake/netxduo/CMakeLists.txt:393) and v6_bsd_raw_build defines NX_BSD_RAW_SUPPORT
(test/cmake/netxduo/CMakeLists.txt:138 and :327), so this is a hard failure in that
configuration rather than just a warning.

Please add const to the two helper declarations and definitions.

While you're there: the casts inside this function now discard const too —
nxd_bsd.c:3838, :3839 and :3852–:3857 cast a const struct nx_bsd_sockaddr * to
struct nx_bsd_sockaddr_in * / ..._in6 *. That is MISRA C 2012 Rule 11.8 (required: a cast
shall not remove a const qualification). They should become
(const struct nx_bsd_sockaddr_in *) etc.

[fdesbiens]

The strict/lenient split itself is a good idea, but it's being applied to the wrong function.

inet_aton() is specified by 4.3BSD and implemented by glibc, musl and the BSDs to accept all
four forms — a, a.b, a.b.c, a.b.c.d. The inet_aton(3) man page documents this
explicitly. inet_pton() is the function that accepts only the full dotted-quad form. So making
inet_aton strict moves away from POSIX/BSD behaviour rather than toward it, and it is a
silent behaviour change for every existing application that relies on the documented forms.

It also makes the two entry points inconsistent with each other: nx_bsd_inet_addr is routed
with NX_FALSE at nxd_bsd.c:6103, so after this change
inet_addr("11.10.2570") succeeds while inet_aton("11.10.2570") fails. On every real platform
inet_addr() is a thin wrapper over inet_aton() and they share one parser, so this divergence
would be surprising.

Suggested change: pass NX_FALSE from inet_aton (keeping today's behaviour) and NX_TRUE
only from the two inet_pton call sites at nxd_bsd.c:13557 and nxd_bsd.c:13666. That also
lets the three test changes in this PR be reverted (see the comments on
netx_bsd_aton_test.c:65 and netx_bsd_recvfromto_test.c:247).

Minor: return 0; should be return(0); to match every other return in this function, and
use_strict_parsing == NX_TRUE is better written as a plain truth test since the parameter is an
INT supplied by internal callers.

[fdesbiens]

This renames a public API function without leaving anything behind under the old name. After this
change no nx_bsd_inet_aton symbol exists anywhere — git grep nx_bsd_inet_aton on the PR head
returns only the _impl spelling.

That breaks applications built with NX_BSD_ENABLE_NATIVE_API. That option is documented and
user-selectable (nxd_bsd.h:117), and when it is defined the override block at nxd_bsd.h:180 is
skipped entirely — so those applications call nx_bsd_inet_aton() directly, and there is no
replacement for them at all, strict or lenient. The new inet_aton macro doesn't help, because
it lives inside the block that native mode compiles out.

Suggestion that avoids all of this: make the strict/lenient worker a static helper (e.g.
nx_bsd_inet_aton_internal(const CHAR *, struct nx_bsd_in_addr *, UINT strict)) and keep
nx_bsd_inet_aton() as the public function, unchanged in signature and behaviour, calling the
helper with the lenient flag. nx_bsd_inet_pton() then calls the helper with the strict flag.
The header needs no change at all, NX_BSD_ENABLE_NATIVE_API keeps working, and the internal
helper stays out of the public surface.

[fdesbiens]

Three problems with this line, all of which disappear if the static helper approach in the
nxd_bsd.h:1007 comment is used instead.

1. It reverses the direction of the block. Every other entry in this section maps the
   internal nx_bsd_* name onto the plain POSIX name (#define nx_bsd_inet_addr inet_addr),
   which is how the nx_bsd_-prefixed definitions in nxd_bsd.c come to be compiled under their
   POSIX names. This line goes the other way, defining inet_aton in terms of an
   nx_bsd_* symbol, so the file now contains two opposite conventions in adjacent lines.

2. It is the only function-like macro in a block of object-like macros. As a result
   inet_aton is no longer usable as a value — &inet_aton, or passing it as a callback, stops
   compiling for application code that previously worked.

3. The macro parameters are not parenthesised. MISRA C 2012 Rule 20.7 (required) requires
   macro parameters to be wrapped in parentheses in the expansion, i.e.
   nx_bsd_inet_aton_impl((address_buffer_ptr), (addr), NX_TRUE).

[fdesbiens]

use_strict_parsing only enforces the number of dot-separated groups. It doesn't restrict the
digit format, so inet_pton(AF_INET, ...) still accepts input that POSIX requires it to reject:

• hex groups — "0xb.0xa.0xa.0xa"
• octal groups and leading zeros — "013.012.012.012", "127.0.0.010"
• trailing whitespace, accepted by the nx_bsd_isspace check at nxd_bsd.c:5966

The existing assertions at netx_bsd_aton_test.c:69-83 confirm the hex and octal forms still
return 1, and those go through the same parser that inet_pton now uses with NX_TRUE.

For strict mode to actually mean "POSIX inet_pton", it also needs to require decimal digits
only, reject a leading 0 on a multi-digit group, and reject trailing whitespace. Worth adding
test cases for each of those, since they're the interesting inputs.

[fdesbiens]

These two assertions were passing before this PR, and they were testing documented inet_aton()
behaviour — the a.b.c and a.b forms are part of the function's contract on every platform
that has it. Rewriting them to expect failure encodes the behaviour change rather than
testing it.

If inet_aton stays lenient as suggested on nxd_bsd.c:6012, this hunk can simply be reverted.
The strict cases belong in the inet_pton tests instead, where rejecting "11.10.2570" is
correct — and ideally alongside the hex/octal/leading-zero cases from the nxd_bsd.c:13557
comment

[fdesbiens]

Same point as on netx_bsd_aton_test.c:65: this block was added specifically to assert that
inet_aton() still accepts the abbreviated forms, and it's now inverted to assert the opposite.
Please revert this hunk along with the inet_aton behaviour.

[fdesbiens]

Tests 4 and 5 cover the two writefds changes nicely, but the other half of this PR — the
exceptfds URG scan in select() (nxd_bsd.c:8380-8400) and the URG check in
nx_bsd_select_wakeup (nxd_bsd.c:10334-10361) — has no test at all. We need
matching regression tests and 100% coverage for new or extended behaviour.

Worth adding at least: a socket in exceptfds only (no readfds) that becomes ready when an
urgent segment arrives, and the negative case where normal data must not set exceptfds — the
latter partly exists as Test 2, but only for the readfds+exceptfds combination.

Note that the strict-parsing branch at nxd_bsd.c:6012 is covered, and the lenient a.b /
a.b.c branches keep their coverage through netx_bsd_inet_addr_pton_test.c:73-77, so no
coverage is lost there.

Also worth confirming: does the unlisten fix at nxd_bsd.c:5011 have a test? The failed-accept
path it depends on is hard to reach, but without one the fix is untested.

[fdesbiens]

The memcmp half of this condition doesn't test anything meaningful. When inet_pton rejects an
address it returns 0 without writing to dst, so ipv4_addr still holds the result of the last
successful call — ipv4_addr_str1 = "1.2.3.4" from line 111. The assertion therefore passes
only because 0x01020304 happens to differ from ipv4_addr_num3 (0x0b0a0a0a); reorder the
fixtures and it would start failing for no real reason.

if (status != 0) error_counter++; is the whole check.

(Unrelated pre-existing bug, since you're in the area: netx_bsd_inet_addr_pton_test.c:111 uses
&& where || was clearly intended, so that assertion can never fail. Fine to leave for a
separate PR.)

[fdesbiens]

Test 5 is defined here and Test 4 about 130 lines further down at line 302, so the file now reads
1, 2, 3, 5, 4. The ordering is forced by where each test has to run relative to accept(), which
is fine — just swap the two numbers so they read in order.