Re: [Linuxptp-devel] [PATCH RFC 28/30] interface: Hide the implementation details.
On 3/4/2020 9:24 AM, Richard Cochran wrote: > On Tue, Feb 18, 2020 at 01:32:01PM -0800, Jacob Keller wrote: >> I'd appreciate if there was some way to ensure we catch the interface >> structure layout changing such that the definitions in clock.c and >> config.c aren't compatible with interface.c anymore. >> >> Perhaps there isn't a good solution for that besides code review? > > I am not too worried about the safety, because moving the list head > will cause the whole thing to explode. But there would be a better > way. The struct port has the same design, and so I'll address this > issue with a follow up series. Ok sounds good. > > Thanks, > Richard > ___ Linuxptp-devel mailing list Linuxptp-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linuxptp-devel
Re: [Linuxptp-devel] [PATCH RFC 28/30] interface: Hide the implementation details.
On Tue, Feb 18, 2020 at 01:32:01PM -0800, Jacob Keller wrote: > I'd appreciate if there was some way to ensure we catch the interface > structure layout changing such that the definitions in clock.c and > config.c aren't compatible with interface.c anymore. > > Perhaps there isn't a good solution for that besides code review? I am not too worried about the safety, because moving the list head will cause the whole thing to explode. But there would be a better way. The struct port has the same design, and so I'll address this issue with a follow up series. Thanks, Richard ___ Linuxptp-devel mailing list Linuxptp-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linuxptp-devel
Re: [Linuxptp-devel] [PATCH RFC 28/30] interface: Hide the implementation details.
On 2/11/2020 6:04 AM, Richard Cochran wrote:
> Now that a complete functional API is in place, there is no need to expose
> the inner workings of the network interface data type. This patch converts
> it into an opaque type while leaving the list marker visible to users
> through a simple form of "friendly exposition".
>
I'd appreciate if there was some way to ensure we catch the interface
structure layout changing such that the definitions in clock.c and
config.c aren't compatible with interface.c anymore.
Perhaps there isn't a good solution for that besides code review?
> Signed-off-by: Richard Cochran
> ---
> clock.c | 4
> config.c| 4
> interface.c | 7 +++
> interface.h | 9 ++---
> nsm.c | 4
> 5 files changed, 21 insertions(+), 7 deletions(-)
>
> diff --git a/clock.c b/clock.c
> index e5f104e..1668383 100644
> --- a/clock.c
> +++ b/clock.c
> @@ -47,6 +47,10 @@
> #define N_CLOCK_PFD (N_POLLFD + 1) /* one extra per port, for the fault
> timer */
> #define POW2_41 ((double)(1ULL << 41))
>
> +struct interface {
> + STAILQ_ENTRY(interface) list;
> +};
> +
I would appreciate some sort of comment here and/or in the .c file which
indicates that list entry must come first.
> struct port {
> LIST_ENTRY(port) list;
> };
> diff --git a/config.c b/config.c
> index e033842..f20c5f7 100644
> --- a/config.c
> +++ b/config.c
> @@ -32,6 +32,10 @@
> #include "print.h"
> #include "util.h"
>
> +struct interface {
> + STAILQ_ENTRY(interface) list;
> +};
> +
> enum config_section {
> GLOBAL_SECTION,
> UC_MTAB_SECTION,
> diff --git a/interface.c b/interface.c
> index 63ed7e4..7cd5b41 100644
> --- a/interface.c
> +++ b/interface.c
> @@ -7,6 +7,13 @@
> #include
> #include "interface.h"
>
> +struct interface {
> + STAILQ_ENTRY(interface) list;
> + char name[MAX_IFNAME_SIZE + 1];
> + char ts_label[MAX_IFNAME_SIZE + 1];
> + struct sk_ts_info ts_info;
> +};
> +
> struct interface *interface_create(const char *name)
> {
> struct interface *iface;
> diff --git a/interface.h b/interface.h
> index b61f4d6..6cc50ac 100644
> --- a/interface.h
> +++ b/interface.h
> @@ -17,13 +17,8 @@
> #error if_namesize larger than expected.
> #endif
>
> -/** Defines a network interface, with PTP options. */
> -struct interface {
> - STAILQ_ENTRY(interface) list;
> - char name[MAX_IFNAME_SIZE + 1];
> - char ts_label[MAX_IFNAME_SIZE + 1];
> - struct sk_ts_info ts_info;
> -};
> +/** Opaque type */
> +struct interface;
>
Is there a way that we can include the "friendly exposition" definition
within this header? I imagine because interface.c includes interface.h
the compiler complains...
> /**
> * Creates an instance of an interface.
> diff --git a/nsm.c b/nsm.c
> index 1292c6b..5aa925b 100644
> --- a/nsm.c
> +++ b/nsm.c
> @@ -35,6 +35,10 @@
> #define IFMT "\n\t\t"
> #define NSM_NFD 3
>
> +struct interface {
> + STAILQ_ENTRY(interface) list;
> +};
> +
> struct nsm {
> struct config *cfg;
> struct fdarray fda;
>
___
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel
[Linuxptp-devel] [PATCH RFC 28/30] interface: Hide the implementation details.
Now that a complete functional API is in place, there is no need to expose
the inner workings of the network interface data type. This patch converts
it into an opaque type while leaving the list marker visible to users
through a simple form of "friendly exposition".
Signed-off-by: Richard Cochran
---
clock.c | 4
config.c| 4
interface.c | 7 +++
interface.h | 9 ++---
nsm.c | 4
5 files changed, 21 insertions(+), 7 deletions(-)
diff --git a/clock.c b/clock.c
index e5f104e..1668383 100644
--- a/clock.c
+++ b/clock.c
@@ -47,6 +47,10 @@
#define N_CLOCK_PFD (N_POLLFD + 1) /* one extra per port, for the fault timer
*/
#define POW2_41 ((double)(1ULL << 41))
+struct interface {
+ STAILQ_ENTRY(interface) list;
+};
+
struct port {
LIST_ENTRY(port) list;
};
diff --git a/config.c b/config.c
index e033842..f20c5f7 100644
--- a/config.c
+++ b/config.c
@@ -32,6 +32,10 @@
#include "print.h"
#include "util.h"
+struct interface {
+ STAILQ_ENTRY(interface) list;
+};
+
enum config_section {
GLOBAL_SECTION,
UC_MTAB_SECTION,
diff --git a/interface.c b/interface.c
index 63ed7e4..7cd5b41 100644
--- a/interface.c
+++ b/interface.c
@@ -7,6 +7,13 @@
#include
#include "interface.h"
+struct interface {
+ STAILQ_ENTRY(interface) list;
+ char name[MAX_IFNAME_SIZE + 1];
+ char ts_label[MAX_IFNAME_SIZE + 1];
+ struct sk_ts_info ts_info;
+};
+
struct interface *interface_create(const char *name)
{
struct interface *iface;
diff --git a/interface.h b/interface.h
index b61f4d6..6cc50ac 100644
--- a/interface.h
+++ b/interface.h
@@ -17,13 +17,8 @@
#error if_namesize larger than expected.
#endif
-/** Defines a network interface, with PTP options. */
-struct interface {
- STAILQ_ENTRY(interface) list;
- char name[MAX_IFNAME_SIZE + 1];
- char ts_label[MAX_IFNAME_SIZE + 1];
- struct sk_ts_info ts_info;
-};
+/** Opaque type */
+struct interface;
/**
* Creates an instance of an interface.
diff --git a/nsm.c b/nsm.c
index 1292c6b..5aa925b 100644
--- a/nsm.c
+++ b/nsm.c
@@ -35,6 +35,10 @@
#define IFMT "\n\t\t"
#define NSM_NFD3
+struct interface {
+ STAILQ_ENTRY(interface) list;
+};
+
struct nsm {
struct config *cfg;
struct fdarray fda;
--
2.20.1
___
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel
