posix: unistd.h: Add gethostname()

[pfalcon]

Per POSIX, gethostname() is declared in unistd.h.

Signed-off-by: Paul Sokolovsky [email protected]

[tgorochowik]

I am probably missing something obvious, so forgive me for asking, but how does this relate to the definition in net/socket.h? Should't it be removed there if you're moving it to the proper posix header?

For reference:

zephyr/include/net/socket.h

Lines 628 to 631 in 774d5e8

	static inline int gethostname(char *buf, size_t len)
	{
	return zsock_gethostname(buf, len);
	}

[pfalcon]

@tgorochowik:

1. As a generic answer, POSIX subsys in Zephyr is in quite weak state, and there's nobody who can just come and "fix" or "finish" it with one change. I anticipate 50-100 incremental changes like this are required to get it into more or less decent shape.
2. As a more specific answer, Zephyr's BSD Socket API was initially developed as a standalone subsystem, with primary usecase of developing new apps for Zephyr using standard API. This usecase remains pertinent. However, not less interesting usecase is porting existing software to Zephyr. And as anyone who tries that will discover, it's very rare that some software uses just BSD Socket API and not other POSIX APIs.

So:

• Definition in net/socket.h should remain, for the initial usecase.
• All BSD socket functionality should be also exposed via standard POSIX headers.
• These 2 usages shouldn't conflict with each other (e.g., being exclusive).
• The way to achieve that is described in answer 1 ("a hundred incremental patches").

[tgorochowik]

@pfalcon thank you for the detailed answer.

These 2 usages shouldn't conflict with each other (e.g., being exclusive).

That is what I mean. Your patch actually makes them conflict with each other.

If you enable all the POSIX stuff in config including sockets and CONFIG_SOCKETS_POSIX_NAMES, you get a redefinition.

So taking into account that this is meant to be a part of many patches, it should be at least wrapped in an ifdef (#if !defined(CONFIG_SOCKETS_POSIX_NAMES) ? ) - at least temporarily.

[pfalcon]

Your patch actually makes them conflict with each other.

CI disagrees. So, please remove your -1, you're blocking next 99 patches ;-). And at the current velocity (based on the #15628 stats), we'll need 4 years to go thru them.

OTOH, if you experience an issue, feel free to join "a hundred patches" streak, submit your patch, and we'll see if it goes in the right direction.

[tgorochowik]

Apparently CI does not test it thoroughly enough, consider the following, example:

$ git diff
diff --git a/samples/hello_world/prj.conf b/samples/hello_world/prj.conf
index b2a4ba5910..666726e540 100644
--- a/samples/hello_world/prj.conf
+++ b/samples/hello_world/prj.conf
@@ -1 +1,9 @@
 # nothing here
+CONFIG_POSIX_API=y
+CONFIG_NETWORKING=y
+CONFIG_NET_HOSTNAME_ENABLE=y
+CONFIG_NET_SOCKETS=y
+CONFIG_NET_SOCKETS_POSIX_NAMES=y

diff --git a/samples/hello_world/src/main.c b/samples/hello_world/src/main.c
index 2d4a7f0d5f..27c71c88b4 100644
--- a/samples/hello_world/src/main.c
+++ b/samples/hello_world/src/main.c
@@ -7,6 +7,9 @@
 #include <zephyr.h>
 #include <misc/printk.h>
 
+#include <net/socket.h>
+#include <posix/unistd.h>
+
 void main(void)
 {
        printk("Hello World! %s\n", CONFIG_BOARD);

The compilation gives:

In file included from /path/zephyr/samples/hello_world/src/main.c:11:
/path/zephyr/include/posix/unistd.h:41:19: error: redefinition of 'gethostname'
 static inline int gethostname(char *buf, size_t len)
                   ^~~~~~~~~~~
In file included from /path/zephyr/samples/hello_world/src/main.c:10:
/path/zephyr/include/net/socket.h:628:19: note: previous definition of 'gethostname' was here
 static inline int gethostname(char *buf, size_t len)

[galak]

• Definition in net/socket.h should remain, for the initial usecase.
• All BSD socket functionality should be also exposed via standard POSIX headers.
• These 2 usages shouldn't conflict with each other (e.g., being exclusive).
• The way to achieve that is described in answer 1 ("a hundred incremental patches").

Is this explicit and if not can we make it? Is it valid to have CONFIG_NET_SOCKETS_POSIX_NAMES and CONFIG_POSIX set? Can we make Kconfig such that they are mutually exclusive?

Also what do we see as the final goal w/regards to the initial usecase? ie will we remove CONFIG_NET_SOCKETS_POSIX_NAMES?

[nashif]

CI disagrees. So, please remove your -1, you're blocking next 99 patches

you probably want to submit all those 99 patches in one PR. Solving the issues 1 patch at a time is going to make reviewing something like this very difficult without understanding the full picture and an idea where we are going with this.

[pfalcon]

@tgorochowik

Apparently CI does not test it thoroughly enough, consider the following, example:

Of course it doesn't, or we'd never get issues like #15618 . Yet we have that and a thousand of other issues. And as you remember from #16009 (comment), there's no giant who can solve them up at once.

The compilation gives:

Patches welcome.

[tgorochowik]

The compilation gives:

Patches welcome.

I am not sure I follow you here. The example I gave compiles just fine without the patch from this PR. So while I agree that there is a lot to be done with POSIX in Zephyr (as you know I encountered some of the issues myself), I don't see why we would want to fix one thing by breaking another (especially that it seems to be fairly simple to avoid, either with the ifdef I suggested or by making the config options exclusive like @galak suggested.)

Feel free to remove my -1 if you don't consider that to be a problem.

[pfalcon]

@galak

Is this explicit and if not can we make it? Is it valid to have CONFIG_NET_SOCKETS_POSIX_NAMES and CONFIG_POSIX set? Can we make Kconfig such that they are mutually exclusive?

In my list, we'll have too, if we haven't yet. Checking and doing that would be subject of one of next hundred of patches. Or I hope @tgorochowik can pick it up.

Also what do we see as the final goal w/regards to the initial usecase? ie will we remove CONFIG_NET_SOCKETS_POSIX_NAMES?

I'm wary of "final goals". I definitely have some vision in my head, but it's pretty clear that it would differ a lot from visions other folks have. The latest example is @nashif's tirade at yesterday's networking forum regarding usage of POSIX API for networking projects. You see, we already use POSIX API (even if part of it, BSD Sockets) for all things networking, and yet when people want to use even more POSIX API, that surprises @nashif. That shows that there's no common vision across project stakeholders, and achieving that would be the mid-way goal before defining the final ones.

Answering specific questions, I personally don't see CONFIG_NET_SOCKETS_POSIX_NAMES going anywhere soon. A prerequisite for considering that would be really-working POSIX subsys, and that's rather distant goal.

[pfalcon]

test

[pfalcon]

@nashif

you probably want to submit all those 99 patches in one PR.

I definitely don't want to do that. And if you want to review PRs like that, you can try #12984 . This patch is nothing but extract from it.

(I also regularly try reviews like that in network subsys, which are of course not really reviewable and after which there's a trail of regressions.)

Solving the issues 1 patch at a time is going to make reviewing something like this very difficult

Contrary, it's very easy - is it a step in the right direction (here: does unistd.h really define gethostname() per POSIX)? Does it pass CI? Then merge, rinse, and repeat.

without understanding the full picture and an idea where we are going with this.

Unfortunately, nobody can understand full picture of everything. That's why there're subsystem maintainers, which have a more or less a full picture of a subset of project. And I'm left as the only POSIX subsys maintainer. Except that I'm not, because an entry in Zephyr's CODEOWNERS doesn't really mean anything like a subsystem maintainer in other projects. That said, I exactly communicate my understanding of the problem and plan to solve it.

Hopefully the upcoming patches will fix the issues I raised

[nashif]

I not fond of introducing technical debt with the promise that it will be fixed in the future. Please submit everything to address this issue in one PR.

[pfalcon]

Not directly related issue of CONFIG_POSIX_API vs CONFIG_SOCKETS_POSIX_NAMES conflict is split into #16141 .

[pfalcon]

@nashif: With #16557 merged, please re-review/merge this PR.

Requested change was implemented in #16557