Skip to content

posix: Add headers related to BSD Sockets API #16621

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Aug 7, 2019
Merged

posix: Add headers related to BSD Sockets API #16621

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 30 additions & 0 deletions include/posix/arpa/inet.h
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
/*
* Copyright (c) 2019 Linaro Limited
*
* SPDX-License-Identifier: Apache-2.0
*/
#ifndef ZEPHYR_INCLUDE_POSIX_ARPA_INET_H_
#define ZEPHYR_INCLUDE_POSIX_ARPA_INET_H_

#ifdef __cplusplus
extern "C" {
#endif

#include <net/socket.h>

static inline char *inet_ntop(sa_family_t family, const void *src, char *dst,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are you sure about this? These two functions don't seem to belong in this file: http://pubs.opengroup.org/onlinepubs/007908799/xns/arpainet.h.html

What SUS version are you targeting here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What SUS version are you targeting here?

Targeting latest publicly available POSIX version: http://pubs.opengroup.org/onlinepubs/9699919799/

http://pubs.opengroup.org/onlinepubs/9699919799/functions/inet_ntop.html

Quick check: "man inet_ntop"

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I haven't been following the posix compatibility work carefully, but this PR makes me wonder if it's just piecemeal efforts to support individual applications, or a disciplined approach that's enabling functionality in a staged way.

I'm not asking for the entire POSIX API to be implemented in one go. That's a ridiculous strawman.

I am asking what the structure of the approach is, given that this PR seems to have problems in that regard.

size_t size)
{
return zsock_inet_ntop(family, src, dst, size);
}

static inline int inet_pton(sa_family_t family, const char *src, void *dst)
{
return zsock_inet_pton(family, src, dst);
}

#ifdef __cplusplus
}
#endif

#endif /* ZEPHYR_INCLUDE_POSIX_ARPA_INET_H_ */
11 changes: 11 additions & 0 deletions include/posix/net/if.h
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
/*
* Copyright (c) 2019 Linaro Limited
*
* SPDX-License-Identifier: Apache-2.0
*/
#ifndef ZEPHYR_INCLUDE_POSIX_NET_IF_H_
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMO, having the header but not actually supporting the features it's specified to contain does more harm than good.

This comment applies multiple places in this PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMO, having the header but not actually supporting the features it's specified to contain does more harm than good.

What makes you think so? Do you want me to implement the whole POSIX (with extensions) in one PR? Sorry, that's not possible.

Otherwise, as the commit message says, "This set of headers is what is required to build
https://github.com/open62541/open62541 with Zephyr." So, without this (empty) header, the POSIX application specimen doesn't build, and with that header, even if empty, it builds. Judge the good.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Otherwise, as the commit message says, "This set of headers is what is required to build
open62541/open62541 with Zephyr." So, without this (empty) header, the POSIX application specimen doesn't build, and with that header, even if empty, it builds. Judge the good.

so you are saying anyone building a 3rd party library/project should start adding empty headers in zephyr to satisfy the external build? This just does not make any sense.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so you are saying anyone building a 3rd party library/project should start adding empty headers in zephyr to satisfy the external build?

Where did I say that? I said that locations and presence of POSIX headers is mandated by the POSIX standard, and 3rd-party software relies on that (and specific example is given). I apologize once again for not putting implementation of the entire POSIX in this PR, and instead just making dozenth small step in the direction of better support of it in Zephyr, like I did before and will intend to do further, for as long as my work supported. (I can easily put it on backburner and do instead things which my boss told me to do ;-) ).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Otherwise, as the commit message says, "This set of headers is what is required to build
https://github.com/open62541/open62541 with Zephyr." So, without this (empty) header, the POSIX application specimen doesn't build, and with that header, even if empty, it builds. Judge the good.

Yes, I saw that.

If you're writing a patch to get this single application to compile, it's not ready for upstream. If Zephyr offers a POSIX header but don't implement or declare literally a single piece of its functionality just to get a single application to compile, it's just false advertising.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you're writing a patch to get this single application to compile, it's not ready for upstream.

No, that's not what I'm doing. See my reply to @nashif. I'm gradually improving POSIX subsystem, period.

If Zephyr offers a POSIX header but don't implement or declare literally a single piece of its functionality just to get a single application to compile, it's just false advertising.

The problem, Marti, is that Zephyr already advertises POSIX subsystem, which doesn't really work. Likewise, it advertises network stack which doesn't really work. That's the things I'm aware of. And I'm not sure about your current company, but I bet your previous one just relies on all that stuff to already work. Likewise, real top-level work I should be doing assumes that all this stuff just works. But it doesn't. I'm a bit shy to tell that "no, Zephyr can't do that at all/reliably" to 3rd-parties, that's why I submit these patches. However, I bet I need to go back to my real assignments for now after all.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The problem, Marti, is that Zephyr already advertises POSIX subsystem, which doesn't really work.

can you please tell or educate me how any of the changes in this patch improve or touch the existing POSIX subsystem?

You are just adding headers that we never supported and never claimed to support. Zephyr never claimed it supports POSIX, our goal was limited to only a subset of PSE52, nothing more. We know there are some issues there, especially with headers and c libraries, but I do not see this fixing any of that, instead, and per your commit message, you are adding empty headers to make an external project build.

Period.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nashif:

can you please tell or educate me how any of the changes in this patch improve or touch the existing POSIX subsystem?

I've been doing that with a number of comments already. If it's still not clear, then perhaps the communication pipe is to narrow and other medium is required.

Zephyr never claimed it supports POSIX, our goal was limited to only a subset of PSE52, nothing more.

I don't know which party you refer to in "our goal". (Intel? Zephyr TSC?) But our goal, as Zephyr users and contributors, is to run existing, real-world software with Zephyr. There're tons of existing, useful, more or less quality software available in the industry, whereas developing new software from scratch takes years, and fixing issues with it takes decades. I'm not sure if "you" (the party you refer to) overlook this fact, or willfully want to force Zephyr user to use NIH, proprietary, misdesigned (just look how many of "native" Zephyr APIs get deprecated, or "new API for X" appear) APIs. In the latter case, we respectfully disagree. In the former case, we're happy to bring it to your attention. (See also #16683 and feel free to ask me for more user stories).

What's more interesting is that we (parties who want the above) were here all the time. For example, all 3 years I'm contributing to Zephyr, I'm working in the direction described above. (And of course, I do that not on my own whim, but as part of my work assignments.) So, it's truly insightful to finally (or for the Nth time?) let know about our existence and say hello.

you are adding empty headers to make an external project build.

I'm adding headers mandated by POSIX (big, real-world POSIX, not some fine-print TLA you put in "your" marketing materials). Yes, for this initial submission some of them empty. They are intended to be filled in gradually and incrementally, in the same way as development of any project goes. And here we switch again to process and community management mattes - either encouraging contributors by valuing their dedicated, even if incremental, work, or discouraging by write-offs like "empty files are BAAAD!!!11" (empty files are bad, most of the time; but sometimes mere presence of a file gives enough significance).

#define ZEPHYR_INCLUDE_POSIX_NET_IF_H_

#include <net/socket.h>

#endif /* ZEPHYR_INCLUDE_POSIX_NET_IF_H_ */
46 changes: 46 additions & 0 deletions include/posix/netdb.h
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
/*
* Copyright (c) 2019 Linaro Limited
*
* SPDX-License-Identifier: Apache-2.0
*/
#ifndef ZEPHYR_INCLUDE_POSIX_NETDB_H_
#define ZEPHYR_INCLUDE_POSIX_NETDB_H_

#ifdef __cplusplus
extern "C" {
#endif

#include <net/socket.h>

#define addrinfo zsock_addrinfo

static inline int getaddrinfo(const char *host, const char *service,
const struct zsock_addrinfo *hints,
struct zsock_addrinfo **res)
{
return zsock_getaddrinfo(host, service, hints, res);
}

static inline void freeaddrinfo(struct zsock_addrinfo *ai)
{
zsock_freeaddrinfo(ai);
}

static inline const char *gai_strerror(int errcode)
{
return zsock_gai_strerror(errcode);
}

static inline int getnameinfo(const struct sockaddr *addr, socklen_t addrlen,
char *host, socklen_t hostlen,
char *serv, socklen_t servlen, int flags)
{
return zsock_getnameinfo(addr, addrlen, host, hostlen,
serv, servlen, flags);
}

#ifdef __cplusplus
}
#endif

#endif /* ZEPHYR_INCLUDE_POSIX_NETDB_H_ */
11 changes: 11 additions & 0 deletions include/posix/netinet/in.h
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
/*
* Copyright (c) 2019 Linaro Limited
*
* SPDX-License-Identifier: Apache-2.0
*/
#ifndef ZEPHYR_INCLUDE_POSIX_NETINET_IN_H_
#define ZEPHYR_INCLUDE_POSIX_NETINET_IN_H_

#include <net/socket.h>

#endif /* ZEPHYR_INCLUDE_POSIX_NETINET_IN_H_ */
11 changes: 11 additions & 0 deletions include/posix/netinet/tcp.h
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
/*
* Copyright (c) 2019 Linaro Limited
*
* SPDX-License-Identifier: Apache-2.0
*/
#ifndef ZEPHYR_INCLUDE_POSIX_NETINET_TCP_H_
#define ZEPHYR_INCLUDE_POSIX_NETINET_TCP_H_

#include <net/socket.h>

#endif /* ZEPHYR_INCLUDE_POSIX_NETINET_TCP_H_ */
11 changes: 11 additions & 0 deletions include/posix/sys/ioctl.h
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
/*
* Copyright (c) 2019 Linaro Limited
*
* SPDX-License-Identifier: Apache-2.0
*/
#ifndef ZEPHYR_INCLUDE_POSIX_SYS_IOCTL_H_
#define ZEPHYR_INCLUDE_POSIX_SYS_IOCTL_H_

#define FIONBIO 0x5421

#endif /* ZEPHYR_INCLUDE_POSIX_SYS_IOCTL_H_ */
90 changes: 90 additions & 0 deletions include/posix/sys/socket.h
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,90 @@
/*
* Copyright (c) 2019 Linaro Limited
*
* SPDX-License-Identifier: Apache-2.0
*/
#ifndef ZEPHYR_INCLUDE_POSIX_SYS_SOCKET_H_
#define ZEPHYR_INCLUDE_POSIX_SYS_SOCKET_H_

#ifdef __cplusplus
extern "C" {
#endif

#include <sys/types.h>
#include <net/socket.h>

static inline int socket(int family, int type, int proto)
{
return zsock_socket(family, type, proto);
}

#define SHUT_RD ZSOCK_SHUT_RD
#define SHUT_WR ZSOCK_SHUT_WR
#define SHUT_RDWR ZSOCK_SHUT_RDWR

static inline int shutdown(int sock, int how)
{
return zsock_shutdown(sock, how);
}

static inline int bind(int sock, const struct sockaddr *addr, socklen_t addrlen)
{
return zsock_bind(sock, addr, addrlen);
}

static inline int connect(int sock, const struct sockaddr *addr,
socklen_t addrlen)
{
return zsock_connect(sock, addr, addrlen);
}

static inline int listen(int sock, int backlog)
{
return zsock_listen(sock, backlog);
}

static inline int accept(int sock, struct sockaddr *addr, socklen_t *addrlen)
{
return zsock_accept(sock, addr, addrlen);
}

static inline ssize_t send(int sock, const void *buf, size_t len, int flags)
{
return zsock_send(sock, buf, len, flags);
}

static inline ssize_t recv(int sock, void *buf, size_t max_len, int flags)
{
return zsock_recv(sock, buf, max_len, flags);
}

static inline ssize_t sendto(int sock, const void *buf, size_t len, int flags,
const struct sockaddr *dest_addr,
socklen_t addrlen)
{
return zsock_sendto(sock, buf, len, flags, dest_addr, addrlen);
}

static inline ssize_t recvfrom(int sock, void *buf, size_t max_len, int flags,
struct sockaddr *src_addr, socklen_t *addrlen)
{
return zsock_recvfrom(sock, buf, max_len, flags, src_addr, addrlen);
}

static inline int getsockopt(int sock, int level, int optname,
void *optval, socklen_t *optlen)
{
return zsock_getsockopt(sock, level, optname, optval, optlen);
}

static inline int setsockopt(int sock, int level, int optname,
const void *optval, socklen_t optlen)
{
return zsock_setsockopt(sock, level, optname, optval, optlen);
}

#ifdef __cplusplus
}
#endif

#endif /* ZEPHYR_INCLUDE_POSIX_SYS_SOCKET_H_ */