From 7b438defe5e544e43aa29a952f2f519413214683 Mon Sep 17 00:00:00 2001 From: Richard Cochran Date: Sun, 23 Aug 2015 13:59:38 +0200 Subject: [PATCH] config: introduce a proper creation method. Now that all of the legacy, open coded configuration fields are gone, we can follow a normal create/destroy pattern for the configuration. This patch add the new method and converts the programs to use it. Signed-off-by: Richard Cochran --- config.c | 18 ++++++++--- config.h | 2 +- phc2sys.c | 83 +++++++++++++++++++++++------------------------- pmc.c | 11 +++---- ptp4l.c | 94 +++++++++++++++++++++++++++---------------------------- 5 files changed, 106 insertions(+), 102 deletions(-) diff --git a/config.c b/config.c index d2ae5fd..9fa2ecb 100644 --- a/config.c +++ b/config.c @@ -614,15 +614,23 @@ void config_init_interface(struct interface *iface, struct config *cfg) sk_get_ts_info(iface->name, &iface->ts_info); } -int config_init(struct config *cfg) +struct config *config_create(void) { char buf[CONFIG_LABEL_SIZE + 8]; struct config_item *ci; + struct config *cfg; int i; + cfg = calloc(1, sizeof(*cfg)); + if (!cfg) { + return NULL; + } + STAILQ_INIT(&cfg->interfaces); + cfg->htab = hash_create(); if (!cfg->htab) { - return -1; + free(cfg); + return NULL; } /* Populate the hash table with global defaults. */ @@ -646,10 +654,11 @@ int config_init(struct config *cfg) goto fail; } } - return 0; + return cfg; fail: hash_destroy(cfg->htab, NULL); - return -1; + free(cfg); + return NULL; } void config_destroy(struct config *cfg) @@ -661,6 +670,7 @@ void config_destroy(struct config *cfg) free(iface); } hash_destroy(cfg->htab, config_item_free); + free(cfg); } double config_get_double(struct config *cfg, const char *section, diff --git a/config.h b/config.h index 6c4d484..8aa7322 100644 --- a/config.h +++ b/config.h @@ -53,7 +53,7 @@ void config_destroy(struct config *cfg); /* New, hash table based methods: */ -int config_init(struct config *cfg); +struct config *config_create(void); double config_get_double(struct config *cfg, const char *section, const char *option); diff --git a/phc2sys.c b/phc2sys.c index 55062b9..35cf6fa 100644 --- a/phc2sys.c +++ b/phc2sys.c @@ -119,7 +119,7 @@ struct node { struct clock *master; }; -static struct config phc2sys_config; +static struct config *phc2sys_config; static int update_pmc(struct node *node, int subscribe); static int clock_handle_leap(struct node *node, struct clock *clock, @@ -224,7 +224,7 @@ static struct clock *clock_add(struct node *node, char *device) } } - c->servo = servo_create(&phc2sys_config, node->servo_type, + c->servo = servo_create(phc2sys_config, node->servo_type, -ppb, max_ppb, 0); servo_sync_interval(c->servo, node->phc_interval); @@ -1225,7 +1225,7 @@ int main(int argc, char *argv[]) struct config *cfg; int autocfg = 0, rt = 0; int c, domain_number = 0, pps_fd = -1; - int r, wait_sync = 0; + int r = -1, wait_sync = 0; int print_level = LOG_INFO, use_syslog = 1, verbose = 0; int ntpshm_segment; double phc_rate, tmp; @@ -1239,10 +1239,10 @@ int main(int argc, char *argv[]) handle_term_signals(); - if (config_init(&phc2sys_config)) { + cfg = phc2sys_config = config_create(); + if (!cfg) { return -1; } - cfg = &phc2sys_config; config_set_double(cfg, "pi_proportional_const", KP); config_set_double(cfg, "pi_integral_const", KI); @@ -1267,7 +1267,7 @@ int main(int argc, char *argv[]) if (pps_fd < 0) { fprintf(stderr, "cannot open '%s': %m\n", optarg); - return -1; + goto end; } break; case 'i': @@ -1286,69 +1286,64 @@ int main(int argc, char *argv[]) } else { fprintf(stderr, "invalid servo name %s\n", optarg); - return -1; + goto end; } break; case 'P': - if (get_arg_val_d(c, optarg, &tmp, 0.0, DBL_MAX)) - return -1; - if (config_set_double(cfg, "pi_proportional_const", tmp)) - return -1; + if (get_arg_val_d(c, optarg, &tmp, 0.0, DBL_MAX) || + config_set_double(cfg, "pi_proportional_const", tmp)) + goto end; break; case 'I': - if (get_arg_val_d(c, optarg, &tmp, 0.0, DBL_MAX)) - return -1; - if (config_set_double(cfg, "pi_integral_const", tmp)) - return -1; + if (get_arg_val_d(c, optarg, &tmp, 0.0, DBL_MAX) || + config_set_double(cfg, "pi_integral_const", tmp)) + goto end; break; case 'S': - if (get_arg_val_d(c, optarg, &tmp, 0.0, DBL_MAX)) - return -1; - if (config_set_double(cfg, "step_threshold", tmp)) - return -1; + if (get_arg_val_d(c, optarg, &tmp, 0.0, DBL_MAX) || + config_set_double(cfg, "step_threshold", tmp)) + goto end; break; case 'F': - if (get_arg_val_d(c, optarg, &tmp, 0.0, DBL_MAX)) - return -1; - if (config_set_double(cfg, "first_step_threshold", tmp)) - return -1; + if (get_arg_val_d(c, optarg, &tmp, 0.0, DBL_MAX) || + config_set_double(cfg, "first_step_threshold", tmp)) + goto end; break; case 'R': if (get_arg_val_d(c, optarg, &phc_rate, 1e-9, DBL_MAX)) - return -1; + goto end; node.phc_interval = 1.0 / phc_rate; break; case 'N': if (get_arg_val_i(c, optarg, &node.phc_readings, 1, INT_MAX)) - return -1; + goto end; break; case 'O': if (get_arg_val_i(c, optarg, &node.sync_offset, INT_MIN, INT_MAX)) - return -1; + goto end; node.forced_sync_offset = -1; break; case 'L': if (get_arg_val_i(c, optarg, &node.sanity_freq_limit, 0, INT_MAX)) - return -1; + goto end; break; case 'M': - if (get_arg_val_i(c, optarg, &ntpshm_segment, INT_MIN, INT_MAX)) - return -1; - if (config_set_int(cfg, "ntpshm_segment", ntpshm_segment)) - return -1; + if (get_arg_val_i(c, optarg, &ntpshm_segment, INT_MIN, INT_MAX) || + config_set_int(cfg, "ntpshm_segment", ntpshm_segment)) + goto end; break; case 'u': if (get_arg_val_ui(c, optarg, &node.stats_max_count, 0, UINT_MAX)) - return -1; + goto end; break; case 'w': wait_sync = 1; break; case 'n': if (get_arg_val_i(c, optarg, &domain_number, 0, 255)) - return -1; + goto end; break; case 'x': node.kernel_leap = 0; @@ -1357,17 +1352,16 @@ int main(int argc, char *argv[]) if (strlen(optarg) > MAX_IFNAME_SIZE) { fprintf(stderr, "path %s too long, max is %d\n", optarg, MAX_IFNAME_SIZE); - return -1; + goto end; } - if (config_set_string(&phc2sys_config, "uds_address", - optarg)) { - return -1; + if (config_set_string(cfg, "uds_address", optarg)) { + goto end; } break; case 'l': if (get_arg_val_i(c, optarg, &print_level, PRINT_LEVEL_MIN, PRINT_LEVEL_MAX)) - return -1; + goto end; break; case 'm': verbose = 1; @@ -1377,9 +1371,11 @@ int main(int argc, char *argv[]) break; case 'v': version_show(stdout); + config_destroy(cfg); return 0; case 'h': usage(progname); + config_destroy(cfg); return 0; default: goto bad_usage; @@ -1414,10 +1410,10 @@ int main(int argc, char *argv[]) print_set_level(print_level); if (autocfg) { - if (init_pmc(&phc2sys_config, &node, domain_number)) - return -1; + if (init_pmc(cfg, &node, domain_number)) + goto end; if (auto_init_ports(&node, rt) < 0) - return -1; + goto end; r = do_loop(&node, 1); goto end; } @@ -1450,7 +1446,7 @@ int main(int argc, char *argv[]) r = -1; if (wait_sync) { - if (init_pmc(&phc2sys_config, &node, domain_number)) + if (init_pmc(cfg, &node, domain_number)) goto end; while (is_running()) { @@ -1489,9 +1485,10 @@ int main(int argc, char *argv[]) end: if (node.pmc) close_pmc(&node); - + config_destroy(cfg); return r; bad_usage: usage(progname); + config_destroy(cfg); return -1; } diff --git a/pmc.c b/pmc.c index f4779ef..261f2a2 100644 --- a/pmc.c +++ b/pmc.c @@ -42,7 +42,6 @@ #define P41 ((double)(1ULL << 41)) static struct pmc *pmc; -static struct config pmc_config; static void do_get_action(int action, int index, char *str); static void do_set_action(int action, int index, char *str); @@ -739,13 +738,14 @@ int main(int argc, char *argv[]) enum transport_type transport_type = TRANS_UDP_IPV4; UInteger8 boundary_hops = 1, domain_number = 0, transport_specific = 0; struct ptp_message *msg; - struct config *cfg = &pmc_config; + struct config *cfg; #define N_FD 2 struct pollfd pollfd[N_FD]; handle_term_signals(); - if (config_init(&pmc_config)) { + cfg = config_create(); + if (!cfg) { return -1; } @@ -782,8 +782,7 @@ int main(int argc, char *argv[]) config_destroy(cfg); return -1; } - if (config_set_string(&pmc_config, "uds_address", - optarg)) { + if (config_set_string(cfg, "uds_address", optarg)) { config_destroy(cfg); return -1; } @@ -828,7 +827,7 @@ int main(int argc, char *argv[]) print_set_syslog(1); print_set_verbose(1); - pmc = pmc_create(&pmc_config, transport_type, iface_name, boundary_hops, + pmc = pmc_create(cfg, transport_type, iface_name, boundary_hops, domain_number, transport_specific, zero_datalen); if (!pmc) { fprintf(stderr, "failed to create pmc\n"); diff --git a/ptp4l.c b/ptp4l.c index d245517..662e6e6 100644 --- a/ptp4l.c +++ b/ptp4l.c @@ -39,10 +39,6 @@ int assume_two_step = 0; -static struct config cfg_settings = { - .interfaces = STAILQ_HEAD_INITIALIZER(cfg_settings.interfaces), -}; - static struct default_ds ptp4l_dds; static void usage(char *progname) @@ -80,10 +76,10 @@ static void usage(char *progname) int main(int argc, char *argv[]) { char *config = NULL, *req_phc = NULL, *progname, *tmp; - int c; + int c, err = -1; struct interface *iface; - struct clock *clock; - struct config *cfg = &cfg_settings; + struct clock *clock = NULL; + struct config *cfg; struct default_ds *dds = &ptp4l_dds; struct defaultDS *ds = &ptp4l_dds.dds; int phc_index = -1, print_level, required_modes = 0; @@ -92,7 +88,8 @@ int main(int argc, char *argv[]) if (handle_term_signals()) return -1; - if (config_init(&cfg_settings)) { + cfg = config_create(); + if (!cfg) { return -1; } @@ -103,62 +100,62 @@ int main(int argc, char *argv[]) switch (c) { case 'A': if (config_set_int(cfg, "delay_mechanism", DM_AUTO)) - return -1; + goto out; break; case 'E': if (config_set_int(cfg, "delay_mechanism", DM_E2E)) - return -1; + goto out; break; case 'P': if (config_set_int(cfg, "delay_mechanism", DM_P2P)) - return -1; + goto out; break; case '2': if (config_set_int(cfg, "network_transport", TRANS_IEEE_802_3)) - return -1; + goto out; break; case '4': if (config_set_int(cfg, "network_transport", TRANS_UDP_IPV4)) - return -1; + goto out; break; case '6': if (config_set_int(cfg, "network_transport", TRANS_UDP_IPV6)) - return -1; + goto out; break; case 'H': if (config_set_int(cfg, "time_stamping", TS_HARDWARE)) - return -1; + goto out; break; case 'S': if (config_set_int(cfg, "time_stamping", TS_SOFTWARE)) - return -1; + goto out; break; case 'L': if (config_set_int(cfg, "time_stamping", TS_LEGACY_HW)) - return -1; + goto out; break; case 'f': config = optarg; break; case 'i': - if (!config_create_interface(optarg, &cfg_settings)) - return -1; + if (!config_create_interface(optarg, cfg)) + goto out; break; case 'p': req_phc = optarg; break; case 's': if (config_set_int(cfg, "slaveOnly", 1)) { - return -1; + goto out; } break; case 'l': if (get_arg_val_i(c, optarg, &print_level, PRINT_LEVEL_MIN, PRINT_LEVEL_MAX)) - return -1; + goto out; config_set_int(cfg, "logging_level", print_level); break; case 'm': @@ -175,14 +172,14 @@ int main(int argc, char *argv[]) return 0; case '?': usage(progname); - return -1; + goto out; default: usage(progname); - return -1; + goto out; } } - if (config && (c = config_read(config, &cfg_settings))) { + if (config && (c = config_read(config, cfg))) { return c; } @@ -208,23 +205,23 @@ int main(int argc, char *argv[]) if (count_char(tmp, ';') != 2 || static_ptp_text_set(&dds->clock_desc.productDescription, tmp)) { fprintf(stderr, "invalid productDescription '%s'.\n", tmp); - return -1; + goto out; } tmp = config_get_string(cfg, NULL, "revisionData"); if (count_char(tmp, ';') != 2 || static_ptp_text_set(&dds->clock_desc.revisionData, tmp)) { fprintf(stderr, "invalid revisionData '%s'.\n", tmp); - return -1; + goto out; } tmp = config_get_string(cfg, NULL, "userDescription"); if (static_ptp_text_set(&dds->clock_desc.userDescription, tmp)) { fprintf(stderr, "invalid userDescription '%s'.\n", tmp); - return -1; + goto out; } tmp = config_get_string(cfg, NULL, "manufacturerIdentity"); if (OUI_LEN != sscanf(tmp, "%hhx:%hhx:%hhx", &oui[0], &oui[1], &oui[2])) { fprintf(stderr, "invalid manufacturerIdentity '%s'.\n", tmp); - return -1; + goto out; } memcpy(dds->clock_desc.manufacturerIdentity, oui, OUI_LEN); @@ -244,7 +241,7 @@ int main(int argc, char *argv[]) ds->flags & DDS_SLAVE_ONLY) { fprintf(stderr, "Cannot mix 1588 slaveOnly with 802.1AS !gmCapable.\n"); - return -1; + goto out; } if (!config_get_int(cfg, NULL, "gmCapable") || ds->flags & DDS_SLAVE_ONLY) { @@ -255,10 +252,10 @@ int main(int argc, char *argv[]) config_set_int(cfg, "sanity_freq_limit", 0); } - if (STAILQ_EMPTY(&cfg_settings.interfaces)) { + if (STAILQ_EMPTY(&cfg->interfaces)) { fprintf(stderr, "no interface specified\n"); usage(progname); - return -1; + goto out; } if (!(ds->flags & DDS_TWO_STEP_FLAG)) { @@ -267,10 +264,10 @@ int main(int argc, char *argv[]) case TS_LEGACY_HW: fprintf(stderr, "one step is only possible " "with hardware time stamping\n"); - return -1; + goto out; case TS_HARDWARE: if (config_set_int(cfg, "time_stamping", TS_ONESTEP)) - return -1; + goto out; break; case TS_ONESTEP: break; @@ -298,19 +295,19 @@ int main(int argc, char *argv[]) /* Init interface configs and check whether timestamping mode is * supported. */ - STAILQ_FOREACH(iface, &cfg_settings.interfaces, list) { - config_init_interface(iface, &cfg_settings); + STAILQ_FOREACH(iface, &cfg->interfaces, list) { + config_init_interface(iface, cfg); if (iface->ts_info.valid && ((iface->ts_info.so_timestamping & required_modes) != required_modes)) { fprintf(stderr, "interface '%s' does not support " "requested timestamping mode.\n", iface->name); - return -1; + goto out; } } /* determine PHC Clock index */ - iface = STAILQ_FIRST(&cfg_settings.interfaces); + iface = STAILQ_FIRST(&cfg->interfaces); if (config_get_int(cfg, NULL, "free_running")) { phc_index = -1; } else if (config_get_int(cfg, NULL, "time_stamping") == TS_SOFTWARE || @@ -319,7 +316,7 @@ int main(int argc, char *argv[]) } else if (req_phc) { if (1 != sscanf(req_phc, "/dev/ptp%d", &phc_index)) { fprintf(stderr, "bad ptp device string\n"); - return -1; + goto out; } } else if (iface->ts_info.valid) { phc_index = iface->ts_info.phc_index; @@ -327,7 +324,7 @@ int main(int argc, char *argv[]) fprintf(stderr, "ptp device not specified and\n" "automatic determination is not\n" "supported. please specify ptp device\n"); - return -1; + goto out; } if (phc_index >= 0) { @@ -336,23 +333,24 @@ int main(int argc, char *argv[]) if (generate_clock_identity(&ds->clockIdentity, iface->name)) { fprintf(stderr, "failed to generate a clock identity\n"); - return -1; + goto out; } - clock = clock_create(&cfg_settings, - phc_index, &cfg_settings.interfaces, - &ptp4l_dds); + clock = clock_create(cfg, phc_index, &cfg->interfaces, &ptp4l_dds); if (!clock) { fprintf(stderr, "failed to create a clock\n"); - return -1; + goto out; } + err = 0; + while (is_running()) { if (clock_poll(clock)) break; } - - clock_destroy(clock); - config_destroy(&cfg_settings); - return 0; +out: + if (clock) + clock_destroy(clock); + config_destroy(cfg); + return err; }