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 <richardcochran@gmail.com>

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,

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);

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))
+			if (get_arg_val_d(c, optarg, &tmp, 0.0, DBL_MAX) ||
-				return -1;
+			    config_set_double(cfg, "pi_proportional_const", tmp))
-			if (config_set_double(cfg, "pi_proportional_const", tmp))
+				goto end;
-				return -1;
 			break;
 		case 'I':
-			if (get_arg_val_d(c, optarg, &tmp, 0.0, DBL_MAX))
+			if (get_arg_val_d(c, optarg, &tmp, 0.0, DBL_MAX) ||
-				return -1;
+			    config_set_double(cfg, "pi_integral_const", tmp))
-			if (config_set_double(cfg, "pi_integral_const", tmp))
+				goto end;
-				return -1;
 			break;
 		case 'S':
-			if (get_arg_val_d(c, optarg, &tmp, 0.0, DBL_MAX))
+			if (get_arg_val_d(c, optarg, &tmp, 0.0, DBL_MAX) ||
-				return -1;
+			    config_set_double(cfg, "step_threshold", tmp))
-			if (config_set_double(cfg, "step_threshold", tmp))
+				goto end;
-				return -1;
 			break;
 		case 'F':
-			if (get_arg_val_d(c, optarg, &tmp, 0.0, DBL_MAX))
+			if (get_arg_val_d(c, optarg, &tmp, 0.0, DBL_MAX) ||
-				return -1;
+			    config_set_double(cfg, "first_step_threshold", tmp))
-			if (config_set_double(cfg, "first_step_threshold", tmp))
+				goto end;
-				return -1;
 			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))
+			if (get_arg_val_i(c, optarg, &ntpshm_segment, INT_MIN, INT_MAX) ||
-				return -1;
+			    config_set_int(cfg, "ntpshm_segment", ntpshm_segment))
-			if (config_set_int(cfg, "ntpshm_segment", ntpshm_segment))
+				goto end;
-				return -1;
 			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",
+			if (config_set_string(cfg, "uds_address", optarg)) {
-					      optarg)) {
+				goto end;
-				return -1;
 			}
 			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))
+		if (init_pmc(cfg, &node, domain_number))
-			return -1;
+			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;
 }

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",
+			if (config_set_string(cfg, "uds_address", optarg)) {
-					      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");

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 clock *clock = NULL;
-	struct config *cfg = &cfg_settings;
+	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))
+			if (!config_create_interface(optarg, cfg))
-				return -1;
+				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) {
+	STAILQ_FOREACH(iface, &cfg->interfaces, list) {
-		config_init_interface(iface, &cfg_settings);
+		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,
+	clock = clock_create(cfg, phc_index, &cfg->interfaces, &ptp4l_dds);
-			     phc_index, &cfg_settings.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;
 	}
-
+out:
+	if (clock)
 		clock_destroy(clock);
-	config_destroy(&cfg_settings);
+	config_destroy(cfg);
-	return 0;
+	return err;
 }