Eliminate the ad-hoc use of global variables in the ts2phc program by
introducing one data structure that incorporates them. This might make
the code more understandable to people coming from a kernel background,
since it resembles the type of data organization used there. It is also
now closer to the data organization of phc2sys, a similar program in
both purpose and implementation.
The reason why this is needed has to do with the ts2phc polymorphism for
a PPS source.
In the next patches, PPS sources will expose a clock structure, which
will be synchronized from the main ts2phc.c.
Not all PPS sources will expose a clock, only the PHC kind will. So the
current object encapsulation model needs to be loosened up a little bit,
because the main ts2phc.c needs to synchronize a list of clocks, list
which is populated by the PPS sinks and by the subset of PPS sources
which are capable of being synchronized.
So instead of having the translation modules of ts2phc communicate
through global variables, let's make struct ts2phc_private the common
working space for the entire program, which is a paradigm that is more
natural.
Signed-off-by: Vladimir Oltean <olte...@gmail.com>
Reviewed-by: Jacob Keller <jacob.e.kel...@intel.com>
---
v4->v5:
- Rebased on top of data structure renames
v3->v4:
- Rebased, no conflicts
v3->v2:
- Amended commit message.
- Replaced full license header in ts2phc.h with simple SPDX.
ts2phc.c | 66 +++++++++++++--------------
ts2phc.h | 23 ++++++++++
ts2phc_pps_sink.c | 113 ++++++++++++++++++++++++++--------------------
ts2phc_pps_sink.h | 10 ++--
4 files changed, 125 insertions(+), 87 deletions(-)
create mode 100644 ts2phc.h
diff --git a/ts2phc.c b/ts2phc.c
index 6004e04effdd..f228351bae3a 100644
--- a/ts2phc.c
+++ b/ts2phc.c
@@ -11,23 +11,20 @@
#include "config.h"
#include "interface.h"
#include "print.h"
-#include "ts2phc_pps_source.h"
-#include "ts2phc_pps_sink.h"
+#include "ts2phc.h"
#include "version.h"
struct interface {
STAILQ_ENTRY(interface) list;
};
-static void ts2phc_cleanup(struct config *cfg, struct ts2phc_pps_source *src)
+static void ts2phc_cleanup(struct ts2phc_private *priv)
{
- ts2phc_pps_sink_cleanup();
- if (src) {
- ts2phc_pps_source_destroy(src);
- }
- if (cfg) {
- config_destroy(cfg);
- }
+ ts2phc_pps_sink_cleanup(priv);
+ if (priv->src)
+ ts2phc_pps_source_destroy(priv->src);
+ if (priv->cfg)
+ config_destroy(priv->cfg);
}
static void usage(char *progname)
@@ -56,8 +53,8 @@ static void usage(char *progname)
int main(int argc, char *argv[])
{
int c, err = 0, have_sink = 0, index, print_level;
- struct ts2phc_pps_source *src = NULL;
enum ts2phc_pps_source_type pps_type;
+ struct ts2phc_private priv = {0};
char *config = NULL, *progname;
const char *pps_source = NULL;
struct config *cfg = NULL;
@@ -68,7 +65,7 @@ int main(int argc, char *argv[])
cfg = config_create();
if (!cfg) {
- ts2phc_cleanup(cfg, src);
+ ts2phc_cleanup(&priv);
return -1;
}
@@ -81,14 +78,14 @@ int main(int argc, char *argv[])
switch (c) {
case 0:
if (config_parse_option(cfg, opts[index].name, optarg))
{
- ts2phc_cleanup(cfg, src);
+ ts2phc_cleanup(&priv);
return -1;
}
break;
case 'c':
if (!config_create_interface(optarg, cfg)) {
fprintf(stderr, "failed to add PPS sink\n");
- ts2phc_cleanup(cfg, src);
+ ts2phc_cleanup(&priv);
return -1;
}
have_sink = 1;
@@ -99,7 +96,7 @@ int main(int argc, char *argv[])
case 'l':
if (get_arg_val_i(c, optarg, &print_level,
PRINT_LEVEL_MIN, PRINT_LEVEL_MAX)) {
- ts2phc_cleanup(cfg, src);
+ ts2phc_cleanup(&priv);
return -1;
}
config_set_int(cfg, "logging_level", print_level);
@@ -116,29 +113,29 @@ int main(int argc, char *argv[])
case 's':
if (pps_source) {
fprintf(stderr, "too many PPS sources\n");
- ts2phc_cleanup(cfg, src);
+ ts2phc_cleanup(&priv);
return -1;
}
pps_source = optarg;
break;
case 'v':
- ts2phc_cleanup(cfg, src);
+ ts2phc_cleanup(&priv);
version_show(stdout);
return 0;
case 'h':
- ts2phc_cleanup(cfg, src);
+ ts2phc_cleanup(&priv);
usage(progname);
return -1;
case '?':
default:
- ts2phc_cleanup(cfg, src);
+ ts2phc_cleanup(&priv);
usage(progname);
return -1;
}
}
if (config && (c = config_read(config, cfg))) {
fprintf(stderr, "failed to read config\n");
- ts2phc_cleanup(cfg, src);
+ ts2phc_cleanup(&priv);
return -1;
}
print_set_progname(progname);
@@ -147,18 +144,21 @@ int main(int argc, char *argv[])
print_set_syslog(config_get_int(cfg, NULL, "use_syslog"));
print_set_level(config_get_int(cfg, NULL, "logging_level"));
+ STAILQ_INIT(&priv.sinks);
+ priv.cfg = cfg;
+
STAILQ_FOREACH(iface, &cfg->interfaces, list) {
if (1 == config_get_int(cfg, interface_name(iface),
"ts2phc.master")) {
if (pps_source) {
fprintf(stderr, "too many PPS sources\n");
- ts2phc_cleanup(cfg, src);
+ ts2phc_cleanup(&priv);
return -1;
}
pps_source = interface_name(iface);
} else {
- if (ts2phc_pps_sink_add(cfg, interface_name(iface))) {
+ if (ts2phc_pps_sink_add(&priv, interface_name(iface))) {
fprintf(stderr, "failed to add PPS sink\n");
- ts2phc_cleanup(cfg, src);
+ ts2phc_cleanup(&priv);
return -1;
}
have_sink = 1;
@@ -166,19 +166,19 @@ int main(int argc, char *argv[])
}
if (!have_sink) {
fprintf(stderr, "no PPS sinks specified\n");
- ts2phc_cleanup(cfg, src);
+ ts2phc_cleanup(&priv);
usage(progname);
return -1;
}
if (!pps_source) {
fprintf(stderr, "no PPS source specified\n");
- ts2phc_cleanup(cfg, src);
+ ts2phc_cleanup(&priv);
usage(progname);
return -1;
}
- if (ts2phc_pps_sink_arm()) {
- fprintf(stderr, "failed to arm PPS sinks\n");
- ts2phc_cleanup(cfg, src);
+ if (ts2phc_pps_sinks_init(&priv)) {
+ fprintf(stderr, "failed to initialize PPS sinks\n");
+ ts2phc_cleanup(&priv);
return -1;
}
@@ -189,21 +189,21 @@ int main(int argc, char *argv[])
} else {
pps_type = TS2PHC_PPS_SOURCE_PHC;
}
- src = ts2phc_pps_source_create(cfg, pps_source, pps_type);
- if (!src) {
+ priv.src = ts2phc_pps_source_create(cfg, pps_source, pps_type);
+ if (!priv.src) {
fprintf(stderr, "failed to create PPS source\n");
- ts2phc_cleanup(cfg, src);
+ ts2phc_cleanup(&priv);
return -1;
}
while (is_running()) {
- err = ts2phc_pps_sink_poll(src);
+ err = ts2phc_pps_sink_poll(&priv);
if (err) {
pr_err("poll failed");
break;
}
}
- ts2phc_cleanup(cfg, src);
+ ts2phc_cleanup(&priv);
return err;
}
diff --git a/ts2phc.h b/ts2phc.h
new file mode 100644
index 000000000000..14cb2b0c21a3
--- /dev/null
+++ b/ts2phc.h
@@ -0,0 +1,23 @@
+/**
+ * @file ts2phc.h
+ * @brief Structure definitions for ts2phc
+ * @note Copyright 2020 Vladimir Oltean <olte...@gmail.com>
+ * @note SPDX-License-Identifier: GPL-2.0+
+ */
+#ifndef HAVE_TS2PHC_H
+#define HAVE_TS2PHC_H
+
+struct ts2phc_sink_array;
+
+struct ts2phc_private {
+ struct ts2phc_pps_source *src;
+ STAILQ_HEAD(sink_ifaces_head, ts2phc_pps_sink) sinks;
+ unsigned int n_sinks;
+ struct ts2phc_sink_array *polling_array;
+ struct config *cfg;
+};
+
+#include "ts2phc_pps_source.h"
+#include "ts2phc_pps_sink.h"
+
+#endif
diff --git a/ts2phc_pps_sink.c b/ts2phc_pps_sink.c
index 278c79e22d65..6a537d08c21b 100644
--- a/ts2phc_pps_sink.c
+++ b/ts2phc_pps_sink.c
@@ -21,8 +21,7 @@
#include "phc.h"
#include "print.h"
#include "servo.h"
-#include "ts2phc_pps_source.h"
-#include "ts2phc_pps_sink.h"
+#include "ts2phc.h"
#include "util.h"
#define NS_PER_SEC 1000000000LL
@@ -47,7 +46,7 @@ struct ts2phc_pps_sink {
struct ts2phc_sink_array {
struct ts2phc_pps_sink **sink;
struct pollfd *pfd;
-} polling_array;
+};
struct ts2phc_source_timestamp {
struct timespec ts;
@@ -65,49 +64,55 @@ ts2phc_pps_sink_offset(struct ts2phc_pps_sink *sink,
struct ts2phc_source_timestamp ts,
int64_t *offset, uint64_t *local_ts);
-static STAILQ_HEAD(pps_sink_ifaces_head, ts2phc_pps_sink) ts2phc_sinks =
- STAILQ_HEAD_INITIALIZER(ts2phc_sinks);
-
-static unsigned int ts2phc_n_sinks;
-
-static int ts2phc_pps_sink_array_create(void)
+static int ts2phc_pps_sink_array_create(struct ts2phc_private *priv)
{
+ struct ts2phc_sink_array *polling_array;
struct ts2phc_pps_sink *sink;
unsigned int i;
- if (polling_array.sink) {
- return 0;
+ polling_array = malloc(sizeof(*polling_array));
+ if (!polling_array) {
+ pr_err("low memory");
+ return -1;
}
- polling_array.sink = malloc(ts2phc_n_sinks *
sizeof(*polling_array.sink));
- if (!polling_array.sink) {
+
+ polling_array->sink = malloc(priv->n_sinks *
+ sizeof(*polling_array->sink));
+ if (!polling_array->sink) {
pr_err("low memory");
return -1;
}
- polling_array.pfd = malloc(ts2phc_n_sinks * sizeof(*polling_array.pfd));
- if (!polling_array.pfd) {
+ polling_array->pfd = malloc(priv->n_sinks *
+ sizeof(*polling_array->pfd));
+ if (!polling_array->pfd) {
pr_err("low memory");
- free(polling_array.sink);
- polling_array.sink = NULL;
+ free(polling_array->sink);
+ polling_array->sink = NULL;
return -1;
}
i = 0;
- STAILQ_FOREACH(sink, &ts2phc_sinks, list) {
- polling_array.sink[i] = sink;
+ STAILQ_FOREACH(sink, &priv->sinks, list) {
+ polling_array->sink[i] = sink;
i++;
}
- for (i = 0; i < ts2phc_n_sinks; i++) {
- polling_array.pfd[i].events = POLLIN | POLLPRI;
- polling_array.pfd[i].fd = polling_array.sink[i]->fd;
+ for (i = 0; i < priv->n_sinks; i++) {
+ polling_array->pfd[i].events = POLLIN | POLLPRI;
+ polling_array->pfd[i].fd = polling_array->sink[i]->fd;
}
+
+ priv->polling_array = polling_array;
+
return 0;
}
-static void ts2phc_pps_sink_array_destroy(void)
+static void ts2phc_pps_sink_array_destroy(struct ts2phc_private *priv)
{
- free(polling_array.sink);
- free(polling_array.pfd);
- polling_array.sink = NULL;
- polling_array.pfd = NULL;
+ struct ts2phc_sink_array *polling_array = priv->polling_array;
+
+ free(polling_array->sink);
+ free(polling_array->pfd);
+ polling_array->sink = NULL;
+ polling_array->pfd = NULL;
}
static int ts2phc_pps_sink_clear_fifo(struct ts2phc_pps_sink *sink)
@@ -143,9 +148,10 @@ static int ts2phc_pps_sink_clear_fifo(struct
ts2phc_pps_sink *sink)
return 0;
}
-static struct ts2phc_pps_sink *ts2phc_pps_sink_create(struct config *cfg,
+static struct ts2phc_pps_sink *ts2phc_pps_sink_create(struct ts2phc_private
*priv,
const char *device)
{
+ struct config *cfg = priv->cfg;
enum servo_type servo = config_get_int(cfg, NULL, "clock_servo");
int err, fadj, junk, max_adj, pulsewidth;
struct ptp_extts_request extts;
@@ -348,28 +354,28 @@ ts2phc_pps_sink_offset(struct ts2phc_pps_sink *sink,
/* public methods */
-int ts2phc_pps_sink_add(struct config *cfg, const char *name)
+int ts2phc_pps_sink_add(struct ts2phc_private *priv, const char *name)
{
struct ts2phc_pps_sink *sink;
/* Create each interface only once. */
- STAILQ_FOREACH(sink, &ts2phc_sinks, list) {
+ STAILQ_FOREACH(sink, &priv->sinks, list) {
if (0 == strcmp(name, sink->name)) {
return 0;
}
}
- sink = ts2phc_pps_sink_create(cfg, name);
+ sink = ts2phc_pps_sink_create(priv, name);
if (!sink) {
pr_err("failed to create sink");
return -1;
}
- STAILQ_INSERT_TAIL(&ts2phc_sinks, sink, list);
- ts2phc_n_sinks++;
+ STAILQ_INSERT_TAIL(&priv->sinks, sink, list);
+ priv->n_sinks++;
return 0;
}
-int ts2phc_pps_sink_arm(void)
+int ts2phc_pps_sink_arm(struct ts2phc_private *priv)
{
struct ptp_extts_request extts;
struct ts2phc_pps_sink *sink;
@@ -377,7 +383,7 @@ int ts2phc_pps_sink_arm(void)
memset(&extts, 0, sizeof(extts));
- STAILQ_FOREACH(sink, &ts2phc_sinks, list) {
+ STAILQ_FOREACH(sink, &priv->sinks, list) {
extts.index = sink->pin_desc.chan;
extts.flags = sink->polarity | PTP_ENABLE_FEATURE;
err = ioctl(sink->fd, PTP_EXTTS_REQUEST2, &extts);
@@ -389,29 +395,38 @@ int ts2phc_pps_sink_arm(void)
return 0;
}
-void ts2phc_pps_sink_cleanup(void)
+int ts2phc_pps_sinks_init(struct ts2phc_private *priv)
+{
+ int err;
+
+ err = ts2phc_pps_sink_array_create(priv);
+ if (err)
+ return err;
+
+ return ts2phc_pps_sink_arm(priv);
+}
+
+void ts2phc_pps_sink_cleanup(struct ts2phc_private *priv)
{
struct ts2phc_pps_sink *sink;
- ts2phc_pps_sink_array_destroy();
+ ts2phc_pps_sink_array_destroy(priv);
- while ((sink = STAILQ_FIRST(&ts2phc_sinks))) {
- STAILQ_REMOVE_HEAD(&ts2phc_sinks, list);
+ while ((sink = STAILQ_FIRST(&priv->sinks))) {
+ STAILQ_REMOVE_HEAD(&priv->sinks, list);
ts2phc_pps_sink_destroy(sink);
- ts2phc_n_sinks--;
+ priv->n_sinks--;
}
}
-int ts2phc_pps_sink_poll(struct ts2phc_pps_source *src)
+int ts2phc_pps_sink_poll(struct ts2phc_private *priv)
{
+ struct ts2phc_sink_array *polling_array = priv->polling_array;
struct ts2phc_source_timestamp source_ts;
unsigned int i;
int cnt, err;
- if (ts2phc_pps_sink_array_create()) {
- return -1;
- }
- cnt = poll(polling_array.pfd, ts2phc_n_sinks, 2000);
+ cnt = poll(polling_array->pfd, priv->n_sinks, 2000);
if (cnt < 0) {
if (EINTR == errno) {
return 0;
@@ -424,12 +439,12 @@ int ts2phc_pps_sink_poll(struct ts2phc_pps_source *src)
return 0;
}
- err = ts2phc_pps_source_getppstime(src, &source_ts.ts);
+ err = ts2phc_pps_source_getppstime(priv->src, &source_ts.ts);
source_ts.valid = err ? false : true;
- for (i = 0; i < ts2phc_n_sinks; i++) {
- if (polling_array.pfd[i].revents & (POLLIN|POLLPRI)) {
- ts2phc_pps_sink_event(polling_array.sink[i], source_ts);
+ for (i = 0; i < priv->n_sinks; i++) {
+ if (polling_array->pfd[i].revents & (POLLIN|POLLPRI)) {
+ ts2phc_pps_sink_event(polling_array->sink[i],
source_ts);
}
}
return 0;
diff --git a/ts2phc_pps_sink.h b/ts2phc_pps_sink.h
index e2518d726c29..5fe2440e0843 100644
--- a/ts2phc_pps_sink.h
+++ b/ts2phc_pps_sink.h
@@ -7,14 +7,14 @@
#ifndef HAVE_TS2PHC_PPS_SINK_H
#define HAVE_TS2PHC_PPS_SINK_H
-#include "ts2phc_pps_source.h"
+#include "ts2phc.h"
-int ts2phc_pps_sink_add(struct config *cfg, const char *name);
+int ts2phc_pps_sink_add(struct ts2phc_private *priv, const char *name);
-int ts2phc_pps_sink_arm(void);
+int ts2phc_pps_sinks_init(struct ts2phc_private *priv);
-void ts2phc_pps_sink_cleanup(void);
+void ts2phc_pps_sink_cleanup(struct ts2phc_private *priv);
-int ts2phc_pps_sink_poll(struct ts2phc_pps_source *src);
+int ts2phc_pps_sink_poll(struct ts2phc_private *priv);
#endif
--
2.25.1
_______________________________________________
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel
