[Linuxptp-devel] [PATCH RFC] Simplify tests on configuration ranges.

[Richard Cochran]

This patch simplifies some expressions which validate that configuration
variables are within the allowed ranges.

Signed-off-by: Richard Cochran <***@gmail.com>
---
config.c | 16 ++++++++--------
1 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/config.c b/config.c
index e056652..f32ab5f 100644
--- a/config.c
+++ b/config.c
@@ -234,7 +234,7 @@ static enum parser_result parse_global_setting(const char *option,
dds->priority2 = u8;

} else if (!strcmp(option, "domainNumber")) {
- if (1 != sscanf(value, "%hhu", &u8) || !(u8 < 128))
+ if (1 != sscanf(value, "%hhu", &u8) || u8 > 127)
return BAD_VALUE;
dds->domainNumber = u8;

@@ -260,7 +260,7 @@ static enum parser_result parse_global_setting(const char *option,
cfg->dds.free_running = val ? 1 : 0;

} else if (!strcmp(option, "freq_est_interval")) {
- if (1 != sscanf(value, "%d", &val) || !(val >= 0))
+ if (1 != sscanf(value, "%d", &val) || val < 0)
return BAD_VALUE;
cfg->dds.freq_est_interval = val;
pod->freq_est_interval = val;
@@ -271,27 +271,27 @@ static enum parser_result parse_global_setting(const char *option,
*cfg->assume_two_step = val ? 1 : 0;

} else if (!strcmp(option, "tx_timestamp_timeout")) {
- if (1 != sscanf(value, "%u", &val) || !(val > 0))
+ if (1 != sscanf(value, "%u", &val) || val <= 0)
return BAD_VALUE;
*cfg->tx_timestamp_timeout = val;

} else if (!strcmp(option, "pi_proportional_const")) {
- if (1 != sscanf(value, "%lf", &df) || !(df >= 0.0 && df < 1.0))
+ if (1 != sscanf(value, "%lf", &df) || df < 0.0 || df >= 1.0)
return BAD_VALUE;
*cfg->pi_proportional_const = df;

} else if (!strcmp(option, "pi_integral_const")) {
- if (1 != sscanf(value, "%lf", &df) || !(df >= 0.0 && df < 1.0))
+ if (1 != sscanf(value, "%lf", &df) || df < 0.0 || df >= 1.0)
return BAD_VALUE;
*cfg->pi_integral_const = df;

} else if (!strcmp(option, "pi_offset_const")) {
- if (1 != sscanf(value, "%lf", &df) || !(df >= 0.0))
+ if (1 != sscanf(value, "%lf", &df) || df < 0.0)
return BAD_VALUE;
*cfg->pi_offset_const = df;

} else if (!strcmp(option, "pi_max_frequency")) {
- if (1 != sscanf(value, "%d", &val) || !(val >= 0))
+ if (1 != sscanf(value, "%d", &val) || val < 0)
return BAD_VALUE;
*cfg->pi_max_frequency = val;

@@ -316,7 +316,7 @@ static enum parser_result parse_global_setting(const char *option,

} else if (!strcmp(option, "logging_level")) {
if (1 != sscanf(value, "%d", &val) ||
- !(val >= PRINT_LEVEL_MIN && val <= PRINT_LEVEL_MAX))
+ val < PRINT_LEVEL_MIN || val > PRINT_LEVEL_MAX)
return BAD_VALUE;
if (!(cfg_ignore & CFG_IGNORE_PRINT_LEVEL)) {
cfg->print_level = val;

[Keller, Jacob E]

-----Original Message-----
Sent: Friday, April 12, 2013 10:39 AM
Subject: [Linuxptp-devel] [PATCH RFC] Simplify tests on configuration
ranges.
This patch simplifies some expressions which validate that configuration
variables are within the allowed ranges.
---

Looks great. Definitely better than we had before.

- Jake

[Miroslav Lichvar]

Post by Richard Cochran
This patch simplifies some expressions which validate that configuration
variables are within the allowed ranges.

For me, the original code is a bit more readable, because I can see
the allowed range directly without having to negate the expression in
my head.

I'm ok with both versions as long as it's used consistently.

Post by Richard Cochran
} else if (!strcmp(option, "pi_proportional_const")) {
- if (1 != sscanf(value, "%lf", &df) || !(df >= 0.0 && df < 1.0))
+ if (1 != sscanf(value, "%lf", &df) || df < 0.0 || df >= 1.0)
return BAD_VALUE;
*cfg->pi_proportional_const = df;
} else if (!strcmp(option, "pi_integral_const")) {
- if (1 != sscanf(value, "%lf", &df) || !(df >= 0.0 && df < 1.0))
+ if (1 != sscanf(value, "%lf", &df) || df < 0.0 || df >= 1.0)
return BAD_VALUE;
*cfg->pi_integral_const = df;

Not related to this patch, but we might need to allow P/I constants
over 1.0 for sub-second update intervals. I'm doing some simulations
and trying to figure out some rules on good P/I. I'll post the results
later.

Thanks,