* [LTP] [RFC PATCH 1/4] lapi: Add lapi/tcp.h with definitions moved from netstress.c
2017-11-09 11:39 [LTP] [RFC PATCH 0/4] Add headers in lapi Petr Vorel
@ 2017-11-09 11:39 ` Petr Vorel
2017-11-09 11:39 ` [LTP] [RFC PATCH 2/4] lapi/socket.h: Move definitions " Petr Vorel
` (3 subsequent siblings)
4 siblings, 0 replies; 13+ messages in thread
From: Petr Vorel @ 2017-11-09 11:39 UTC (permalink / raw)
To: ltp
Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
include/lapi/tcp.h | 25 +++++++++++++++++++++++++
testcases/network/netstress/netstress.c | 6 +-----
2 files changed, 26 insertions(+), 5 deletions(-)
create mode 100644 include/lapi/tcp.h
diff --git a/include/lapi/tcp.h b/include/lapi/tcp.h
new file mode 100644
index 000000000..44d75ab7b
--- /dev/null
+++ b/include/lapi/tcp.h
@@ -0,0 +1,25 @@
+/*
+ * Copyright (c) 2017 Petr Vorel <pvorel@suse.cz>
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#ifndef TCP_H__
+#define TCP_H__
+
+#ifndef TCP_FASTOPEN
+#define TCP_FASTOPEN 23
+#endif
+
+#endif /* TCP_H__ */
diff --git a/testcases/network/netstress/netstress.c b/testcases/network/netstress/netstress.c
index 4b211aa75..d45c858dc 100644
--- a/testcases/network/netstress/netstress.c
+++ b/testcases/network/netstress/netstress.c
@@ -33,6 +33,7 @@
#include <errno.h>
#include "lapi/posix_clocks.h"
+#include "lapi/tcp.h"
#include "tst_safe_pthread.h"
#include "tst_test.h"
@@ -51,11 +52,6 @@ static const int max_msg_len = (1 << 16) - 1;
#define DCCP_SOCKOPT_SERVICE 2
#endif
-/* TCP server requiers */
-#ifndef TCP_FASTOPEN
-#define TCP_FASTOPEN 23
-#endif
-
#ifndef SO_BUSY_POLL
#define SO_BUSY_POLL 46
#endif
--
2.15.0
^ permalink raw reply related [flat|nested] 13+ messages in thread* [LTP] [RFC PATCH 2/4] lapi/socket.h: Move definitions from netstress.c
2017-11-09 11:39 [LTP] [RFC PATCH 0/4] Add headers in lapi Petr Vorel
2017-11-09 11:39 ` [LTP] [RFC PATCH 1/4] lapi: Add lapi/tcp.h with definitions moved from netstress.c Petr Vorel
@ 2017-11-09 11:39 ` Petr Vorel
2017-11-09 15:17 ` Cyril Hrubis
2017-11-10 13:14 ` Alexey Kodanev
2017-11-09 11:39 ` [LTP] [RFC PATCH 3/4] lapi: Add lapi/in.h with definition moved " Petr Vorel
` (2 subsequent siblings)
4 siblings, 2 replies; 13+ messages in thread
From: Petr Vorel @ 2017-11-09 11:39 UTC (permalink / raw)
To: ltp
+ remove sys/socket.h from netstress.c
Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
include/lapi/socket.h | 18 +++++++++++++++++-
testcases/network/netstress/netstress.c | 18 ++----------------
2 files changed, 19 insertions(+), 17 deletions(-)
diff --git a/include/lapi/socket.h b/include/lapi/socket.h
index 98263105e..af9448804 100644
--- a/include/lapi/socket.h
+++ b/include/lapi/socket.h
@@ -19,8 +19,24 @@
#ifndef __LAPI_SOCKET_H__
#define __LAPI_SOCKET_H__
+#ifndef MSG_FASTOPEN
+#define MSG_FASTOPEN 0x20000000 /* Send data in TCP SYN */
+#endif
+
+#ifndef SO_BUSY_POLL
+#define SO_BUSY_POLL 46
+#endif
+
#ifndef SOCK_CLOEXEC
-# define SOCK_CLOEXEC 02000000
+#define SOCK_CLOEXEC 02000000
+#endif
+
+#ifndef SOCK_DCCP
+#define SOCK_DCCP 6
+#endif
+
+#ifndef SOL_DCCP
+#define SOL_DCCP 269
#endif
#endif /* __LAPI_SOCKET_H__ */
diff --git a/testcases/network/netstress/netstress.c b/testcases/network/netstress/netstress.c
index d45c858dc..790a76e20 100644
--- a/testcases/network/netstress/netstress.c
+++ b/testcases/network/netstress/netstress.c
@@ -22,7 +22,6 @@
#include <stdlib.h>
#include <limits.h>
#include <sys/types.h>
-#include <sys/socket.h>
#include <netdb.h>
#include <netinet/in.h>
#include <arpa/inet.h>
@@ -33,34 +32,21 @@
#include <errno.h>
#include "lapi/posix_clocks.h"
+#include "lapi/socket.h"
#include "lapi/tcp.h"
#include "tst_safe_pthread.h"
#include "tst_test.h"
static const int max_msg_len = (1 << 16) - 1;
-#ifndef SOCK_DCCP
-#define SOCK_DCCP 6
-#endif
#ifndef IPPROTO_DCCP
#define IPPROTO_DCCP 33
#endif
-#ifndef SOL_DCCP
-#define SOL_DCCP 269
-#endif
+
#ifndef DCCP_SOCKOPT_SERVICE
#define DCCP_SOCKOPT_SERVICE 2
#endif
-#ifndef SO_BUSY_POLL
-#define SO_BUSY_POLL 46
-#endif
-
-/* TCP client requiers */
-#ifndef MSG_FASTOPEN
-#define MSG_FASTOPEN 0x20000000 /* Send data in TCP SYN */
-#endif
-
enum {
SERVER_HOST = 0,
CLIENT_HOST,
--
2.15.0
^ permalink raw reply related [flat|nested] 13+ messages in thread* [LTP] [RFC PATCH 2/4] lapi/socket.h: Move definitions from netstress.c
2017-11-09 11:39 ` [LTP] [RFC PATCH 2/4] lapi/socket.h: Move definitions " Petr Vorel
@ 2017-11-09 15:17 ` Cyril Hrubis
2017-11-10 13:14 ` Alexey Kodanev
1 sibling, 0 replies; 13+ messages in thread
From: Cyril Hrubis @ 2017-11-09 15:17 UTC (permalink / raw)
To: ltp
Hi!
> #ifndef SOCK_CLOEXEC
> -# define SOCK_CLOEXEC 02000000
> +#define SOCK_CLOEXEC 02000000
> +#endif
FYI the prefered way how to indent innner macros are spaces after the
hash #.
--
Cyril Hrubis
chrubis@suse.cz
^ permalink raw reply [flat|nested] 13+ messages in thread
* [LTP] [RFC PATCH 2/4] lapi/socket.h: Move definitions from netstress.c
2017-11-09 11:39 ` [LTP] [RFC PATCH 2/4] lapi/socket.h: Move definitions " Petr Vorel
2017-11-09 15:17 ` Cyril Hrubis
@ 2017-11-10 13:14 ` Alexey Kodanev
2017-11-21 11:45 ` Petr Vorel
1 sibling, 1 reply; 13+ messages in thread
From: Alexey Kodanev @ 2017-11-10 13:14 UTC (permalink / raw)
To: ltp
On 11/09/2017 02:39 PM, Petr Vorel wrote:
> + remove sys/socket.h from netstress.c
What about including it in lapi/socket.h?
> Signed-off-by: Petr Vorel <pvorel@suse.cz>
> ---
> include/lapi/socket.h | 18 +++++++++++++++++-
> testcases/network/netstress/netstress.c | 18 ++----------------
> 2 files changed, 19 insertions(+), 17 deletions(-)
>
> diff --git a/include/lapi/socket.h b/include/lapi/socket.h
> index 98263105e..af9448804 100644
> --- a/include/lapi/socket.h
> +++ b/include/lapi/socket.h
> @@ -19,8 +19,24 @@
> #ifndef __LAPI_SOCKET_H__
> #define __LAPI_SOCKET_H__
>
> +#ifndef MSG_FASTOPEN
> +#define MSG_FASTOPEN 0x20000000 /* Send data in TCP SYN */
> +#endif
> +
> +#ifndef SO_BUSY_POLL
> +#define SO_BUSY_POLL 46
> +#endif
> +
> #ifndef SOCK_CLOEXEC
> -# define SOCK_CLOEXEC 02000000
> +#define SOCK_CLOEXEC 02000000
> +#endif
> +
> +#ifndef SOCK_DCCP
> +#define SOCK_DCCP 6
> +#endif
The two above should probably be in ascending order...
SOCK_CLOEXEC along with SOCK_NONBLOCK already defined in
lapi/fcntl.h. I guess we should move them to lapi/socket.h instead.
Thanks,
Alexey
^ permalink raw reply [flat|nested] 13+ messages in thread
* [LTP] [RFC PATCH 2/4] lapi/socket.h: Move definitions from netstress.c
2017-11-10 13:14 ` Alexey Kodanev
@ 2017-11-21 11:45 ` Petr Vorel
0 siblings, 0 replies; 13+ messages in thread
From: Petr Vorel @ 2017-11-21 11:45 UTC (permalink / raw)
To: ltp
> On 11/09/2017 02:39 PM, Petr Vorel wrote:
> > + remove sys/socket.h from netstress.c
> What about including it in lapi/socket.h?
> > Signed-off-by: Petr Vorel <pvorel@suse.cz>
> > ---
> > include/lapi/socket.h | 18 +++++++++++++++++-
> > testcases/network/netstress/netstress.c | 18 ++----------------
> > 2 files changed, 19 insertions(+), 17 deletions(-)
> > diff --git a/include/lapi/socket.h b/include/lapi/socket.h
> > index 98263105e..af9448804 100644
> > --- a/include/lapi/socket.h
> > +++ b/include/lapi/socket.h
> > @@ -19,8 +19,24 @@
> > #ifndef __LAPI_SOCKET_H__
> > #define __LAPI_SOCKET_H__
> > +#ifndef MSG_FASTOPEN
> > +#define MSG_FASTOPEN 0x20000000 /* Send data in TCP SYN */
> > +#endif
> > +
> > +#ifndef SO_BUSY_POLL
> > +#define SO_BUSY_POLL 46
> > +#endif
> > +
> > #ifndef SOCK_CLOEXEC
> > -# define SOCK_CLOEXEC 02000000
> > +#define SOCK_CLOEXEC 02000000
> > +#endif
> > +
> > +#ifndef SOCK_DCCP
> > +#define SOCK_DCCP 6
> > +#endif
> The two above should probably be in ascending order...
Thanks, I'll fix it.
> SOCK_CLOEXEC along with SOCK_NONBLOCK already defined in
> lapi/fcntl.h. I guess we should move them to lapi/socket.h instead.
Good point. include/lapi/fcntl.h defines:
#ifndef O_CLOEXEC
# define O_CLOEXEC 02000000
#endif
#ifndef SOCK_CLOEXEC
# define SOCK_CLOEXEC O_CLOEXEC
#endif
This actually makes sense as these both have the same value on the same architecture, but
in glibc headers are these defined separately (in /usr/include/bits/socket_type.h and
/usr/include/asm-generic/fcntl.h).
Both musl and uclibc-ng use the same approach, so I propose to split these definitions in
according to (g)libc headers.
Kind regards,
Petr
^ permalink raw reply [flat|nested] 13+ messages in thread
* [LTP] [RFC PATCH 3/4] lapi: Add lapi/in.h with definition moved from netstress.c
2017-11-09 11:39 [LTP] [RFC PATCH 0/4] Add headers in lapi Petr Vorel
2017-11-09 11:39 ` [LTP] [RFC PATCH 1/4] lapi: Add lapi/tcp.h with definitions moved from netstress.c Petr Vorel
2017-11-09 11:39 ` [LTP] [RFC PATCH 2/4] lapi/socket.h: Move definitions " Petr Vorel
@ 2017-11-09 11:39 ` Petr Vorel
2017-11-09 15:18 ` Cyril Hrubis
2017-11-09 11:39 ` [LTP] [RFC PATCH 4/4] lapi: Add lapi/dccp.h " Petr Vorel
2017-11-09 14:54 ` [LTP] [RFC PATCH 0/4] Add headers in lapi Cyril Hrubis
4 siblings, 1 reply; 13+ messages in thread
From: Petr Vorel @ 2017-11-09 11:39 UTC (permalink / raw)
To: ltp
+ remove now unused include netinet/in.h
Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
include/lapi/in.h | 25 +++++++++++++++++++++++++
testcases/network/netstress/netstress.c | 6 +-----
2 files changed, 26 insertions(+), 5 deletions(-)
create mode 100644 include/lapi/in.h
diff --git a/include/lapi/in.h b/include/lapi/in.h
new file mode 100644
index 000000000..9e623fa61
--- /dev/null
+++ b/include/lapi/in.h
@@ -0,0 +1,25 @@
+/*
+ * Copyright (c) 2017 Petr Vorel <pvorel@suse.cz>
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#ifndef IN_H__
+#define IN_H__
+
+#ifndef IPPROTO_DCCP
+#define IPPROTO_DCCP 33
+#endif
+
+#endif /* IN_H__ */
diff --git a/testcases/network/netstress/netstress.c b/testcases/network/netstress/netstress.c
index 790a76e20..8a9736a71 100644
--- a/testcases/network/netstress/netstress.c
+++ b/testcases/network/netstress/netstress.c
@@ -23,7 +23,6 @@
#include <limits.h>
#include <sys/types.h>
#include <netdb.h>
-#include <netinet/in.h>
#include <arpa/inet.h>
#include <poll.h>
#include <time.h>
@@ -31,6 +30,7 @@
#include <unistd.h>
#include <errno.h>
+#include "lapi/in.h"
#include "lapi/posix_clocks.h"
#include "lapi/socket.h"
#include "lapi/tcp.h"
@@ -39,10 +39,6 @@
static const int max_msg_len = (1 << 16) - 1;
-#ifndef IPPROTO_DCCP
-#define IPPROTO_DCCP 33
-#endif
-
#ifndef DCCP_SOCKOPT_SERVICE
#define DCCP_SOCKOPT_SERVICE 2
#endif
--
2.15.0
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [LTP] [RFC PATCH 3/4] lapi: Add lapi/in.h with definition moved from netstress.c
2017-11-09 11:39 ` [LTP] [RFC PATCH 3/4] lapi: Add lapi/in.h with definition moved " Petr Vorel
@ 2017-11-09 15:18 ` Cyril Hrubis
2017-11-09 16:05 ` Petr Vorel
0 siblings, 1 reply; 13+ messages in thread
From: Cyril Hrubis @ 2017-11-09 15:18 UTC (permalink / raw)
To: ltp
> --- /dev/null
> +++ b/include/lapi/in.h
> @@ -0,0 +1,25 @@
> +/*
> + * Copyright (c) 2017 Petr Vorel <pvorel@suse.cz>
> + *
> + * This program is free software: you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation, either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program. If not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +#ifndef IN_H__
> +#define IN_H__
> +
> +#ifndef IPPROTO_DCCP
> +#define IPPROTO_DCCP 33
> +#endif
> +
> +#endif /* IN_H__ */
Maybe we should rename this file to netinet_in.h or create netinet
directory in lapi. Including just lapi/in.h is a bit confusing.
--
Cyril Hrubis
chrubis@suse.cz
^ permalink raw reply [flat|nested] 13+ messages in thread
* [LTP] [RFC PATCH 3/4] lapi: Add lapi/in.h with definition moved from netstress.c
2017-11-09 15:18 ` Cyril Hrubis
@ 2017-11-09 16:05 ` Petr Vorel
2017-11-21 11:51 ` Petr Vorel
0 siblings, 1 reply; 13+ messages in thread
From: Petr Vorel @ 2017-11-09 16:05 UTC (permalink / raw)
To: ltp
> Maybe we should rename this file to netinet_in.h or create netinet
> directory in lapi. Including just lapi/in.h is a bit confusing.
Is it because in.h is also in bits/ directory (not only in linux and some glibc directory)?
/usr/include/bits/in.h
/usr/include/netinet/in.h
/usr/include/linux/in.h
As the same we have with lapi/tcp.h (in both kernel and glibc)...
/usr/include/netinet/tcp.h
/usr/include/linux/tcp.h
And I'm for directory.
Kind regards,
Petr
^ permalink raw reply [flat|nested] 13+ messages in thread
* [LTP] [RFC PATCH 3/4] lapi: Add lapi/in.h with definition moved from netstress.c
2017-11-09 16:05 ` Petr Vorel
@ 2017-11-21 11:51 ` Petr Vorel
0 siblings, 0 replies; 13+ messages in thread
From: Petr Vorel @ 2017-11-21 11:51 UTC (permalink / raw)
To: ltp
Hi,
> > Maybe we should rename this file to netinet_in.h or create netinet
> > directory in lapi. Including just lapi/in.h is a bit confusing.
> Is it because in.h is also in bits/ directory (not only in linux and some glibc directory)?
> /usr/include/bits/in.h
> /usr/include/netinet/in.h
> /usr/include/linux/in.h
> As the same we have with lapi/tcp.h (in both kernel and glibc)...
> /usr/include/netinet/tcp.h
> /usr/include/linux/tcp.h
> And I'm for directory.
Actually I'd be for just plain "in.h" as other fileas in lapi/ directory don't prefix
anything, aren't in any special directory.
But if you really don't like that, I'll put it into netinet_in.h.
Kind regards,
Petr
^ permalink raw reply [flat|nested] 13+ messages in thread
* [LTP] [RFC PATCH 4/4] lapi: Add lapi/dccp.h with definition moved from netstress.c
2017-11-09 11:39 [LTP] [RFC PATCH 0/4] Add headers in lapi Petr Vorel
` (2 preceding siblings ...)
2017-11-09 11:39 ` [LTP] [RFC PATCH 3/4] lapi: Add lapi/in.h with definition moved " Petr Vorel
@ 2017-11-09 11:39 ` Petr Vorel
2017-11-09 14:54 ` [LTP] [RFC PATCH 0/4] Add headers in lapi Cyril Hrubis
4 siblings, 0 replies; 13+ messages in thread
From: Petr Vorel @ 2017-11-09 11:39 UTC (permalink / raw)
To: ltp
Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
include/lapi/dccp.h | 25 +++++++++++++++++++++++++
testcases/network/netstress/netstress.c | 5 +----
2 files changed, 26 insertions(+), 4 deletions(-)
create mode 100644 include/lapi/dccp.h
diff --git a/include/lapi/dccp.h b/include/lapi/dccp.h
new file mode 100644
index 000000000..e4b19f240
--- /dev/null
+++ b/include/lapi/dccp.h
@@ -0,0 +1,25 @@
+/*
+ * Copyright (c) 2017 Petr Vorel <pvorel@suse.cz>
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#ifndef DCCP_H__
+#define DCCP_H__
+
+#ifndef DCCP_SOCKOPT_SERVICE
+#define DCCP_SOCKOPT_SERVICE 2
+#endif
+
+#endif /* DCCP_H__ */
diff --git a/testcases/network/netstress/netstress.c b/testcases/network/netstress/netstress.c
index 8a9736a71..53a43fbef 100644
--- a/testcases/network/netstress/netstress.c
+++ b/testcases/network/netstress/netstress.c
@@ -30,6 +30,7 @@
#include <unistd.h>
#include <errno.h>
+#include "lapi/dccp.h"
#include "lapi/in.h"
#include "lapi/posix_clocks.h"
#include "lapi/socket.h"
@@ -39,10 +40,6 @@
static const int max_msg_len = (1 << 16) - 1;
-#ifndef DCCP_SOCKOPT_SERVICE
-#define DCCP_SOCKOPT_SERVICE 2
-#endif
-
enum {
SERVER_HOST = 0,
CLIENT_HOST,
--
2.15.0
^ permalink raw reply related [flat|nested] 13+ messages in thread* [LTP] [RFC PATCH 0/4] Add headers in lapi
2017-11-09 11:39 [LTP] [RFC PATCH 0/4] Add headers in lapi Petr Vorel
` (3 preceding siblings ...)
2017-11-09 11:39 ` [LTP] [RFC PATCH 4/4] lapi: Add lapi/dccp.h " Petr Vorel
@ 2017-11-09 14:54 ` Cyril Hrubis
2017-11-09 15:49 ` Petr Vorel
4 siblings, 1 reply; 13+ messages in thread
From: Cyril Hrubis @ 2017-11-09 14:54 UTC (permalink / raw)
To: ltp
Hi!
> I posted this originally in "netstress: Add imports for TCP_FASTOPEN definition" [1], got reply from Alexey [2].
>
> Do we want to include system headers which originaly define definitions in lapi
> headers? (e.g. include <linux/falloc.h> in include/lapi/fallocate.h as it
> defines FALLOC_FL_KEEP_SIZE from linux/falloc.h)?
> Some of them do it, some not.
I think that it's a bit cleaner to include the system header before we
define the fallback definitions, since that way we are sure to pick the
system ones even if test writer forgets to include the system header in
the test.
But be wary of combining the linux and glibc headers, sometimes that
causes collisions and all kinds of problems.
In the case of fallocate the bits/fcntl-linux.h which is included by
bits/fcntl.h which is included from fcntl.h defines these constants (on
older glibcs, newer include linux/fallocate.h in bits/fcntl-linux.h), so
including both fcntl.h and linux/falloc.h will cause macro
redefinitions in certain cases...
> If yes, do we want to check presence of them with autotools?
Most of the time we do not have to, since these headers are present
everywhere. I do not think that we should be adding autoconf checks just
in case. So until there is a breakage we should assume that check is not
needed.
> What is the purpose of lapi anyway? I suppose just to backport missing features
> added in recent glibc to older versions or missing in other libc (musl, uclibc,
> binder).
The purpose is to have fallback definitions in a single place,
previously these were repeated over and over in each testcase, and
expecially with syscall wrappers that sometimes differ sublty between
architectures this was quite complicated to maintain.
--
Cyril Hrubis
chrubis@suse.cz
^ permalink raw reply [flat|nested] 13+ messages in thread* [LTP] [RFC PATCH 0/4] Add headers in lapi
2017-11-09 14:54 ` [LTP] [RFC PATCH 0/4] Add headers in lapi Cyril Hrubis
@ 2017-11-09 15:49 ` Petr Vorel
0 siblings, 0 replies; 13+ messages in thread
From: Petr Vorel @ 2017-11-09 15:49 UTC (permalink / raw)
To: ltp
Hi Cyril,
thanks for info!
> I think that it's a bit cleaner to include the system header before we
> define the fallback definitions, since that way we are sure to pick the
> system ones even if test writer forgets to include the system header in
> the test.
OK. And so it's proposed to include system headers even when using lapi headers.
This looks to me as a duplicity, but probably good practise.
> But be wary of combining the linux and glibc headers, sometimes that
> causes collisions and all kinds of problems.
> In the case of fallocate the bits/fcntl-linux.h which is included by
> bits/fcntl.h which is included from fcntl.h defines these constants (on
> older glibcs, newer include linux/fallocate.h in bits/fcntl-linux.h), so
> including both fcntl.h and linux/falloc.h will cause macro
> redefinitions in certain cases...
OK. IMHO most of the time glibc headers are enough (it's a question what will be newer in
old distro: glibc header or kernel one).
> > If yes, do we want to check presence of them with autotools?
> Most of the time we do not have to, since these headers are present
> everywhere. I do not think that we should be adding autoconf checks just
> in case. So until there is a breakage we should assume that check is not
> needed.
Right!
> > What is the purpose of lapi anyway? I suppose just to backport missing features
> > added in recent glibc to older versions or missing in other libc (musl, uclibc,
> > binder).
> The purpose is to have fallback definitions in a single place,
> previously these were repeated over and over in each testcase, and
> expecially with syscall wrappers that sometimes differ sublty between
> architectures this was quite complicated to maintain.
Sure, I understand DRY principle. I just wasn't sure of the case in
testcases/network/netstress/netstress.c where no system header was used (none of
linux/tcp.h and netinet/tcp.h) and macro was defined, but missing header was probably
left accidentally. Checking that TCP_FASTOPEN was added in 2.18, so it's not in older
distros and in current RHEL (so it's valid to be in lapi header).
Kind regards,
Petr
^ permalink raw reply [flat|nested] 13+ messages in thread