Re: [PATCH] MINOR: resolvers, cfgparse, warn for incorrect 'timeout retry' keyword spelling

[Willy Tarreau]

Hi Pieter,

On Tue, Feb 16, 2016 at 09:42:24PM +0100, P.Baauw wrote:
> Hi Willy, Baptiste,
> 
> Hereby a 'backport' for 1.6.
> Hope its ok like this?
> I took a little different structure as i did for 1.7. But as there is no 
> need to allow different keywords 'in the future' for 1.6 i think it is ok.

Definitely!

Just a few comments below, but don't worry I'll adjust them by myself when
applying :

> + else if (strcmp(args[1], "retry") != 0) {
> + Alert("parsing [%s:%d] : '%s' expects 'retry' and 
>  as arguments got '%s'.\n",
> + file, linenum, args[0], args[1]);
> + err_code |= ERR_ALERT | ERR_WARN;

Normally we don't set ALERT and WARN together, ERR_ALERT reports that a
message was sent using Alert() while ERR_WARN reports that a message was
sent with Warning().

Also given that some people ignore warnings, I tend to indicate when
something is known to be deprecated, I'll probably add something like
"This will not work in future versions".

Thanks!
Willy

[PATCH] MINOR: resolvers, cfgparse, warn for incorrect 'timeout retry' keyword spelling

[P.Baauw]

Hi Willy, Baptiste,

Hereby a 'backport' for 1.6.
Hope its ok like this?
I took a little different structure as i did for 1.7. But as there is no 
need to allow different keywords 'in the future' for 1.6 i think it is ok.


Regards,
Pieter
From a8346de18704ef283621aabf4cb2edc3ebb0ce6d Mon Sep 17 00:00:00 2001
From: PiBa-NL 
Date: Tue, 16 Feb 2016 21:33:02 +0100
Subject: [PATCH] MINOR: resolvers, cfgparse, warn for incorrect 'timeout
 retry' keyword spelling

If for example it was written as 'timeout retri 1s' or 'timeout wrong 1s' this 
would be used for the retry timeout value. Resolvers section only timeout 
setting currently is 'retry', others are still parsed as before this patch to 
not break existing configurations.

This should only be applied as a backport to 1.6, for 1.7 a different patch is 
provided to abort if a wrong keyword is used.
---
 src/cfgparse.c | 8 ++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/src/cfgparse.c b/src/cfgparse.c
index 693e9f3..b06fd2f 100644
--- a/src/cfgparse.c
+++ b/src/cfgparse.c
@@ -2356,13 +2356,17 @@ int cfg_parse_resolvers(const char *file, int linenum, 
char **args, int kwm)
else if (strcmp(args[0], "timeout") == 0) {
const char *res;
unsigned int timeout_retry;
-
-   if (!*args[2]) {
+   if (!*args[1] || !*args[2]) {
Alert("parsing [%s:%d] : '%s' expects 'retry' and 
 as arguments.\n",
file, linenum, args[0]);
err_code |= ERR_ALERT | ERR_FATAL;
goto out;
}
+   else if (strcmp(args[1], "retry") != 0) {
+   Alert("parsing [%s:%d] : '%s' expects 'retry' and 
 as arguments got '%s'.\n",
+   file, linenum, args[0], args[1]);
+   err_code |= ERR_ALERT | ERR_WARN;
+   }
res = parse_time_err(args[2], &timeout_retry, TIME_UNIT_MS);
if (res) {
Alert("parsing [%s:%d]: unexpected character '%c' in 
argument to <%s>.\n",
-- 
2.7.0.windows.1