Re: [PATCH] CLEANUP: assorted typo fixes in the code and comments

[Willy Tarreau]

On Thu, Feb 22, 2024 at 10:12:27AM +0100, Ilya Shipitsin wrote:
> This is 39th iteration of typo fixes

Now merged, thank you! I split it in two because the one on
resolvers and stick-tables directly affects the code (it renames
a function argument) and I want to make it easy to drop it in case
backports need/don't need it.

Thanks,
Willy

[PATCH] CLEANUP: assorted typo fixes in the code and comments

[Ilya Shipitsin]

This is 39th iteration of typo fixes
---
 addons/promex/README  |  4 ++--
 addons/promex/include/promex/promex.h |  4 ++--
 doc/DeviceAtlas-device-detection.txt  |  2 +-
 doc/configuration.txt |  2 +-
 reg-tests/ssl/ocsp_auto_update.vtc| 10 +-
 src/mux_quic.c|  4 ++--
 src/quic_cc_cubic.c   |  2 +-
 src/resolvers.c   |  2 +-
 src/stick_table.c |  2 +-
 9 files changed, 16 insertions(+), 16 deletions(-)

diff --git a/addons/promex/README b/addons/promex/README
index 7f638a5db..8c2266f69 100644
--- a/addons/promex/README
+++ b/addons/promex/README
@@ -81,9 +81,9 @@ It is possible to filter metrics dumped by the exporter. To 
to so, multiple
 "metrics" parameters may be passed to specify all metrics to include or 
exclude,
 as a comma-separated list of filter. By default, there is no filter and all
 metrics are dumped. By specifying at least one metric to be included in the
-dump, this disables the default behavior and only explicitly mentionned metrics
+dump, this disables the default behavior and only explicitly mentioned metrics
 are dumped. To include a metric, its name must be specified. To exclude it, its
-name must be preceeded by a minus character ('-'). Here are examples:
+name must be preceded by a minus character ('-'). Here are examples:
 
   # Dumped all metrics, except "haproxy_server_check_status"
   /metrics?metrics=-haproxy_server_check_status
diff --git a/addons/promex/include/promex/promex.h 
b/addons/promex/include/promex/promex.h
index c4712bc21..74ea2f120 100644
--- a/addons/promex/include/promex/promex.h
+++ b/addons/promex/include/promex/promex.h
@@ -84,13 +84,13 @@ struct promex_label {
  *   is responsible to deinit the dump context.
  *
  * * metric_info(): This one is mandatory. It returns the info about the
- *  metric: name, type and flags and descrition.
+ *  metric: name, type and flags and description.
  *
  * * start_ts(): This one is mandatory, it initializes the context for a 
time
  *   series for a given metric. This context is the second
  *   restart point.
  *
- ** next_ts(): This one is mandatory. It interates on time series for a
+ ** next_ts(): This one is mandatory. It iterates on time series for a
  * given metrics. It is also responsible to handle end of a
  * time series and deinit the context.
  *
diff --git a/doc/DeviceAtlas-device-detection.txt 
b/doc/DeviceAtlas-device-detection.txt
index 2f7ed9f71..9df978377 100644
--- a/doc/DeviceAtlas-device-detection.txt
+++ b/doc/DeviceAtlas-device-detection.txt
@@ -16,7 +16,7 @@ directory. Also, in the case the api cache support is not 
needed and/or a C++ to
 
 However, if the API had been installed beforehand, DEVICEATLAS_SRC
 can be omitted. Note that the DeviceAtlas C API version supported is from the 
3.x
-releases serie (3.2.1 minimum recommended).
+releases series (3.2.1 minimum recommended).
 
 For HAProxy developers who need to verify that their changes didn't 
accidentally
 break the DeviceAtlas code, it is possible to build a dummy library provided in
diff --git a/doc/configuration.txt b/doc/configuration.txt
index 1b35e0141..281ff4176 100644
--- a/doc/configuration.txt
+++ b/doc/configuration.txt
@@ -15317,7 +15317,7 @@ wait-for-body time  [ at-least  ]
 case HAProxy will respond with a 408 "Request Timeout" error to the client
 and stop processing the request. Note that if any of the other conditions
 happens first, this timeout will not occur even if the full body has
-not yet been recieved.
+not yet been received.
 
   This action may be used as a replacement for "option http-buffer-request".
 
diff --git a/reg-tests/ssl/ocsp_auto_update.vtc 
b/reg-tests/ssl/ocsp_auto_update.vtc
index 2ab4a4a08..46d11ea09 100644
--- a/reg-tests/ssl/ocsp_auto_update.vtc
+++ b/reg-tests/ssl/ocsp_auto_update.vtc
@@ -621,11 +621,11 @@ shell {
 
 haproxy h7 -wait
 
-
-#  #
-# EIGTH TEST CASE  #
-#  #
-
+#
+#   #
+# EIGHTH TEST CASE  #
+#   #
+#
 
 #
 # Check that a certificate created through the CLI and which does not have ocsp
diff --git a/src/mux_quic.c b/src/mux_quic.c
index af678b72a..4afebbd8c 100644
--- a/src/mux_quic.c
+++ b/src/mux_quic.c
@@ -1103,7 +1103,7 @@ void qcc_reset_stream(struct qcs *qcs, int err)
/* Soft offset cannot be inferior to real one. */
BUG_ON(qcc->tx.fc.off_soft - diff < qcc->tx.fc.off_real);
 
-   /* Substract to conn flow control data amount prepared on 
stream not yet sent. */
+   /* Subtract to conn flow control data amount prepared on stream 
not yet sent. */
qcc->tx.fc.off_soft -= diff;
 

Re: [PATCH] CLEANUP: log: deinitialization of the log buffer in one function

[Willy Tarreau]

Hi Miroslav,

On Tue, Jan 30, 2024 at 03:42:20AM +0100, Miroslav Zagorac wrote:
> Hello all,
> 
> In several places in the source, there was the same block of code that was
> used to deinitialize the log buffer.  There were even two functions that
> did this, but they were called only from the code that is in the same
> source file (free_tcpcheck_fmt() in src/tcpcheck.c and free_logformat_list()
> in src/proxy.c - they were both static functions).
> 
> The function free_logformat_list() was moved from the file src/proxy.c to
> src/log.c, and a check of the list before freeing the memory was added to
> that function.

Wow, nice catch, great job, you removed 10 list walks on cleanup paths!

   9 files changed, 43 insertions(+), 110 deletions(-)

And regarding the binary:

  $ size haproxy-master haproxy-miroslav 
 textdata bss dec hex filename
  8160770  592116 2666204 11419090 ae3dd2 haproxy-master
  8160592  592116 2666204 11418912 ae3d20 haproxy-miroslav

I love such patches :-)

Thank you!
Willy

[PATCH] CLEANUP: log: deinitialization of the log buffer in one function

[Miroslav Zagorac]

Hello all,

In several places in the source, there was the same block of code that was
used to deinitialize the log buffer.  There were even two functions that
did this, but they were called only from the code that is in the same
source file (free_tcpcheck_fmt() in src/tcpcheck.c and free_logformat_list()
in src/proxy.c - they were both static functions).

The function free_logformat_list() was moved from the file src/proxy.c to
src/log.c, and a check of the list before freeing the memory was added to
that function.

Best regards,

-- 
Miroslav Zagorac
Senior DeveloperFrom 52ef575d77a47ef7338004fbad31d8199e59c752 Mon Sep 17 00:00:00 2001
From: Miroslav Zagorac 
Date: Tue, 30 Jan 2024 03:14:09 +0100
Subject: [PATCH] CLEANUP: log: deinitialization of the log buffer in one
 function

In several places in the source, there was the same block of code that was
used to deinitialize the log buffer.  There were even two functions that
did this, but they were called only from the code that is in the same
source file (free_tcpcheck_fmt() in src/tcpcheck.c and free_logformat_list()
in src/proxy.c - they were both static functions).

The function free_logformat_list() was moved from the file src/proxy.c to
src/log.c, and a check of the list before freeing the memory was added to
that function.
---
 include/haproxy/log.h |  3 +++
 src/fcgi-app.c| 11 +--
 src/http_act.c| 28 
 src/http_htx.c| 26 --
 src/http_rules.c  |  9 +
 src/log.c | 16 
 src/proxy.c   | 12 
 src/tcpcheck.c| 39 +--
 src/vars.c|  9 +
 9 files changed, 43 insertions(+), 110 deletions(-)

diff --git a/include/haproxy/log.h b/include/haproxy/log.h
index 68b820734..9534eaeb2 100644
--- a/include/haproxy/log.h
+++ b/include/haproxy/log.h
@@ -64,6 +64,9 @@ void syslog_fd_handler(int fd);
 int init_log_buffers(void);
 void deinit_log_buffers(void);
 
+/* Deinitialize log buffers used for syslog messages */
+void free_logformat_list(struct list *fmt);
+
 /* build a log line for the session and an optional stream */
 int sess_build_logline(struct session *sess, struct stream *s, char *dst, size_t maxsize, struct list *list_format);
 
diff --git a/src/fcgi-app.c b/src/fcgi-app.c
index 00562f80d..dcd7fd224 100644
--- a/src/fcgi-app.c
+++ b/src/fcgi-app.c
@@ -134,16 +134,7 @@ static void fcgi_release_rule(struct fcgi_rule *rule)
 	if (!rule)
 		return;
 
-	if (!LIST_ISEMPTY(>value)) {
-		struct logformat_node *lf, *lfb;
-
-		list_for_each_entry_safe(lf, lfb, >value, list) {
-			LIST_DELETE(>list);
-			release_sample_expr(lf->expr);
-			free(lf->arg);
-			free(lf);
-		}
-	}
+	free_logformat_list(>value);
 	/* ->cond and ->name are not owned by the rule */
 	free(rule);
 }
diff --git a/src/http_act.c b/src/http_act.c
index 260fe1d67..ebc2bcc68 100644
--- a/src/http_act.c
+++ b/src/http_act.c
@@ -46,17 +46,10 @@
  */
 static void release_http_action(struct act_rule *rule)
 {
-	struct logformat_node *lf, *lfb;
-
 	istfree(>arg.http.str);
 	if (rule->arg.http.re)
 		regex_free(rule->arg.http.re);
-	list_for_each_entry_safe(lf, lfb, >arg.http.fmt, list) {
-		LIST_DELETE(>list);
-		release_sample_expr(lf->expr);
-		free(lf->arg);
-		free(lf);
-	}
+	free_logformat_list(>arg.http.fmt);
 }
 
 /* Release memory allocated by HTTP actions relying on an http reply. Concretly,
@@ -1901,23 +1894,10 @@ static enum act_return http_action_set_map(struct act_rule *rule, struct proxy *
 /* Release memory allocated by an http map/acl action. */
 static void release_http_map(struct act_rule *rule)
 {
-	struct logformat_node *lf, *lfb;
-
 	free(rule->arg.map.ref);
-	list_for_each_entry_safe(lf, lfb, >arg.map.key, list) {
-		LIST_DELETE(>list);
-		release_sample_expr(lf->expr);
-		free(lf->arg);
-		free(lf);
-	}
-	if (rule->action == 1) {
-		list_for_each_entry_safe(lf, lfb, >arg.map.value, list) {
-			LIST_DELETE(>list);
-			release_sample_expr(lf->expr);
-			free(lf->arg);
-			free(lf);
-		}
-	}
+	free_logformat_list(>arg.map.key);
+	if (rule->action == 1)
+		free_logformat_list(>arg.map.value);
 }
 
 /* Parse a "add-acl", "del-acl", "set-map" or "del-map" actions. It takes one or
diff --git a/src/http_htx.c b/src/http_htx.c
index 004d3439a..954473db3 100644
--- a/src/http_htx.c
+++ b/src/http_htx.c
@@ -1117,7 +1117,6 @@ int http_str_to_htx(struct buffer *buf, struct ist raw, char **errmsg)
 
 void release_http_reply(struct http_reply *http_reply)
 {
-	struct logformat_node *lf, *lfb;
 	struct http_reply_hdr *hdr, *hdrb;
 
 	if (!http_reply)
@@ -1126,12 +1125,7 @@ void release_http_reply(struct http_reply *http_reply)
 	ha_free(_reply->ctype);
 	list_for_each_entry_safe(hdr, hdrb, _reply->hdrs, list) {
 		LIST_DELETE(>list);
-		lis

Re: [PATCH] CLEANUP: Re-apply xalloc_size.cocci (3)

[Willy Tarreau]

On Sun, Nov 05, 2023 at 08:02:37PM +0100, Tim Duesterhus wrote:
> This reapplies the xalloc_size.cocci patch across the whole `src/` tree.

Applied, thank you Tim!
Willy

[PATCH] CLEANUP: Re-apply xalloc_size.cocci (3)

[Tim Duesterhus]

This reapplies the xalloc_size.cocci patch across the whole `src/` tree.

see 16cc16dd8235e7eb6c38b7abd210bd1e1d96b1d9
see 63ee0e4c01b94aee5fc6c6dd98cfc4480ae5ea46
see 9fb57e8c175a0b852b06a0780f48eb8eaf321a47
---
 src/log.c| 3 ++-
 src/proto_quic.c | 2 +-
 src/server.c | 4 ++--
 src/tcpcheck.c   | 2 +-
 4 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/src/log.c b/src/log.c
index 8ada05089..3458f93f9 100644
--- a/src/log.c
+++ b/src/log.c
@@ -895,7 +895,8 @@ static int postcheck_log_backend(struct proxy *be)
/* alloc srv array (it will be used for active and backup server lists 
in turn,
 * so we ensure that the longest list will fit
 */
-   be->lbprm.log.srv = calloc(MAX(be->srv_act, be->srv_bck), sizeof(struct 
server *));
+   be->lbprm.log.srv = calloc(MAX(be->srv_act, be->srv_bck),
+  sizeof(*be->lbprm.log.srv));
 
if (!be->lbprm.log.srv ) {
memprintf(, "memory error when allocating server array (%d 
entries)",
diff --git a/src/proto_quic.c b/src/proto_quic.c
index 701dcb295..899cffebf 100644
--- a/src/proto_quic.c
+++ b/src/proto_quic.c
@@ -760,7 +760,7 @@ static int quic_alloc_dghdlrs(void)
MT_LIST_INIT(>dgrams);
}
 
-   quic_cid_trees = calloc(QUIC_CID_TREES_CNT, sizeof(struct 
quic_cid_tree));
+   quic_cid_trees = calloc(QUIC_CID_TREES_CNT, sizeof(*quic_cid_trees));
if (!quic_cid_trees) {
ha_alert("Failed to allocate global CIDs trees.\n");
return 0;
diff --git a/src/server.c b/src/server.c
index 6e9e19564..ca48f2875 100644
--- a/src/server.c
+++ b/src/server.c
@@ -1363,7 +1363,7 @@ static int srv_parse_set_proxy_v2_tlv_fmt(char **args, 
int *cur_arg,
}
}
 
-   srv_tlv = malloc(sizeof(struct srv_pp_tlv_list));
+   srv_tlv = malloc(sizeof(*srv_tlv));
if (unlikely(!srv_tlv)) {
memprintf(err, "'%s' : failed to parse allocate TLV entry", 
args[*cur_arg]);
goto fail;
@@ -2516,7 +2516,7 @@ void srv_settings_cpy(struct server *srv, const struct 
server *src, int srv_tmpl
list_for_each_entry(srv_tlv, >pp_tlvs, list) {
if (srv_tlv == NULL)
break;
-   new_srv_tlv = malloc(sizeof(struct srv_pp_tlv_list));
+   new_srv_tlv = malloc(sizeof(*new_srv_tlv));
if (unlikely(!new_srv_tlv)) {
break;
}
diff --git a/src/tcpcheck.c b/src/tcpcheck.c
index c36b9d9a6..c00c47fed 100644
--- a/src/tcpcheck.c
+++ b/src/tcpcheck.c
@@ -4270,7 +4270,7 @@ int proxy_parse_smtpchk_opt(char **args, int cur_arg, 
struct proxy *curpx, const
(strcmp(args[cur_arg], "EHLO") == 0 || strcmp(args[cur_arg], 
"HELO") == 0)) {
/*  + space (1) +  + null byte (1) */
size_t len = strlen(args[cur_arg]) + 1 + 
strlen(args[cur_arg+1]) + 1;
-   cmd = calloc(len, 1);
+   cmd = calloc(1, len);
if (cmd)
snprintf(cmd, len, "%s %s", args[cur_arg], 
args[cur_arg+1]);
}
-- 
2.42.0

Re: [PATCH] cleanup: remove redundant check

[Willy Tarreau]

On Wed, May 10, 2023 at 04:30:58PM +0200,  ??? wrote:
> Hello,
> 
> small clean patch.
> mutes coverity finding.

Thanks Ilya, now merged in dev11.

Willy

[PATCH] cleanup: remove redundant check

[Илья Шипицин]

Hello,

small clean patch.
mutes coverity finding.

Ilya
From 4fdccb44933c2a91c7d6711bf821cc8b1d4c6d30 Mon Sep 17 00:00:00 2001
From: Ilya Shipitsin 
Date: Wed, 26 Apr 2023 21:05:12 +0200
Subject: [PATCH 1/2] CLEANUP: src/listener.c: remove redundant NULL check

fixes #2031

quoting Willy Tarreau:

"Originally the listeners were intended to work without a bind_conf
(e.g. for FTP processing) hence these tests, but over time the
bind_conf has become omnipresent"
---
 src/listener.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/listener.c b/src/listener.c
index d5390ed85..3da01df21 100644
--- a/src/listener.c
+++ b/src/listener.c
@@ -160,7 +160,7 @@ struct task *accept_queue_process(struct task *t, void 
*context, unsigned int st
if (!(li->bind_conf->options & BC_O_UNLIMITED)) {
HA_ATOMIC_UPDATE_MAX(_max,
 
update_freq_ctr(_per_sec, 1));
-   if (li->bind_conf && li->bind_conf->options & 
BC_O_USE_SSL) {
+   if (li->bind_conf->options & BC_O_USE_SSL) {
HA_ATOMIC_UPDATE_MAX(_max,
 
update_freq_ctr(_per_sec, 1));
}
-- 
2.39.2.windows.1

Re: [PATCH] CLEANUP: use "offsetof" macro where appropriate

[Willy Tarreau]

Hi Ilya,

On Sat, Apr 15, 2023 at 11:55:14PM +0200,  ??? wrote:
> From: Ilya Shipitsin 
> Date: Sat, 15 Apr 2023 23:39:43 +0200
> Subject: [PATCH] CLEANUP: use "offsetof" where appropriate
> 
> let's use the C library macro "offsetof"

Good point. In the past we didn't because it was not always defined, but
now we define it and already use it at a few other places, so let's use
it.

Applied, thanks!
Willy

[PATCH] CLEANUP: use "offsetof" macro where appropriate

[Илья Шипицин]

Hello,

small cleanup patch attached.

Ilya
From 77babd04c417709bb41c951701d62dec0574eb35 Mon Sep 17 00:00:00 2001
From: Ilya Shipitsin 
Date: Sat, 15 Apr 2023 23:39:43 +0200
Subject: [PATCH] CLEANUP: use "offsetof" where appropriate

let's use the C library macro "offsetof"
---
 src/cache.c| 4 ++--
 src/ssl_sock.c | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/src/cache.c b/src/cache.c
index 39e947820..4deb34ea8 100644
--- a/src/cache.c
+++ b/src/cache.c
@@ -425,12 +425,12 @@ static void delete_entry(struct cache_entry *del_entry)
 
 static inline struct shared_context *shctx_ptr(struct cache *cache)
 {
-	return (struct shared_context *)((unsigned char *)cache - ((struct shared_context *)NULL)->data);
+	return (struct shared_context *)((unsigned char *)cache -  offsetof(struct shared_context, data));
 }
 
 static inline struct shared_block *block_ptr(struct cache_entry *entry)
 {
-	return (struct shared_block *)((unsigned char *)entry - ((struct shared_block *)NULL)->data);
+	return (struct shared_block *)((unsigned char *)entry - offsetof(struct shared_block, data));
 }
 
 
diff --git a/src/ssl_sock.c b/src/ssl_sock.c
index abbcfa6af..740fc0aeb 100644
--- a/src/ssl_sock.c
+++ b/src/ssl_sock.c
@@ -4205,7 +4205,7 @@ static inline void sh_ssl_sess_free_blocks(struct shared_block *first, struct sh
 /* return first block from sh_ssl_sess  */
 static inline struct shared_block *sh_ssl_sess_first_block(struct sh_ssl_sess_hdr *sh_ssl_sess)
 {
-	return (struct shared_block *)((unsigned char *)sh_ssl_sess - ((struct shared_block *)NULL)->data);
+	return (struct shared_block *)((unsigned char *)sh_ssl_sess - offsetof(struct shared_block, data));
 
 }
 
-- 
2.40.0

Re: [PATCH] CLEANUP: fix a typo in an error message of http_str_to_htx

[Willy Tarreau]

On Mon, Jan 09, 2023 at 01:31:06AM +, Manu Nicolas wrote:
> This fixes a typo in an error message about headers in the
> http_str_to_htx function.

Applied, thank you Manu!
Willy

[PATCH] CLEANUP: fix a typo in an error message of http_str_to_htx

[Manu Nicolas]

This fixes a typo in an error message about headers in the
http_str_to_htx function.
---
 src/http_htx.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/http_htx.c b/src/http_htx.c
index 2978f2eb4..58b24183f 100644
--- a/src/http_htx.c
+++ b/src/http_htx.c
@@ -924,7 +924,7 @@ int http_str_to_htx(struct buffer *buf, struct ist raw, 
char **errmsg)
ret = h1_headers_to_hdr_list(raw.ptr, istend(raw),
 hdrs, sizeof(hdrs)/sizeof(hdrs[0]), , 
);
if (ret <= 0) {
-   memprintf(errmsg, "unabled to parse headers (error offset: 
%d)", h1m.err_pos);
+   memprintf(errmsg, "unable to parse headers (error offset: %d)", 
h1m.err_pos);
goto error;
}
 
-- 
2.34.1

Re: [PATCH] CLEANUP: Re-apply xalloc_size.cocci (2)

[Christopher Faulet]

Le 6/1/22 à 21:58, Tim Duesterhus a écrit :

This reapplies the xalloc_size.cocci patch across the whole `src/` tree.



Merged. Thanks !

--
Christopher Faulet

Re: [PATCH] CLEANUP: Re-apply xalloc_size.cocci (2)

[Amaury Denoyelle]

On Wed, Jun 01, 2022 at 09:58:37PM +0200, Tim Duesterhus wrote:
> This reapplies the xalloc_size.cocci patch across the whole `src/` tree.
> see 16cc16dd8235e7eb6c38b7abd210bd1e1d96b1d9
> see 63ee0e4c01b94aee5fc6c6dd98cfc4480ae5ea46
> ---
>  src/ncbuf.c  | 2 +-
>  src/proto_quic.c | 2 +-
>  src/quic_sock.c  | 3 ++-
>  3 files changed, 4 insertions(+), 3 deletions(-)
> diff --git a/src/ncbuf.c b/src/ncbuf.c
> index 1944cfe34..adb32b57a 100644
> --- a/src/ncbuf.c
> +++ b/src/ncbuf.c
> @@ -726,7 +726,7 @@ struct rand_off {
>  static struct rand_off *ncb_generate_rand_off(const struct ncbuf *buf)
>  {
>   struct rand_off *roff;
> - roff = calloc(1, sizeof(struct rand_off));
> + roff = calloc(1, sizeof(*roff));
>   BUG_ON(!roff);
>  
>   roff->off = rand() % (ncb_size(buf));
> diff --git a/src/proto_quic.c b/src/proto_quic.c
> index 55aa4b50f..ab1bef18f 100644
> --- a/src/proto_quic.c
> +++ b/src/proto_quic.c
> @@ -703,7 +703,7 @@ static int quic_alloc_dghdlrs(void)
>  {
>   int i;
>  
> - quic_dghdlrs = calloc(global.nbthread, sizeof(struct quic_dghdlr));
> + quic_dghdlrs = calloc(global.nbthread, sizeof(*quic_dghdlrs));
>   if (!quic_dghdlrs) {
>   ha_alert("Failed to allocate the quic datagram handlers.\n");
>   return 0;
> diff --git a/src/quic_sock.c b/src/quic_sock.c
> index 6207af703..a391006af 100644
> --- a/src/quic_sock.c
> +++ b/src/quic_sock.c
> @@ -466,7 +466,8 @@ static int quic_alloc_accept_queues(void)
>  {
>   int i;
>  
> - quic_accept_queues = calloc(global.nbthread, sizeof(struct 
> quic_accept_queue));
> + quic_accept_queues = calloc(global.nbthread,
> + sizeof(*quic_accept_queues));
>   if (!quic_accept_queues) {
>   ha_alert("Failed to allocate the quic accept queues.\n");
>   return 0;
> -- 
> 2.36.1
> 

Fine for me,
Thanks,

-- 
Amaury Denoyelle

[PATCH] CLEANUP: Re-apply xalloc_size.cocci (2)

[Tim Duesterhus]

This reapplies the xalloc_size.cocci patch across the whole `src/` tree.

see 16cc16dd8235e7eb6c38b7abd210bd1e1d96b1d9
see 63ee0e4c01b94aee5fc6c6dd98cfc4480ae5ea46
---
 src/ncbuf.c  | 2 +-
 src/proto_quic.c | 2 +-
 src/quic_sock.c  | 3 ++-
 3 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/src/ncbuf.c b/src/ncbuf.c
index 1944cfe34..adb32b57a 100644
--- a/src/ncbuf.c
+++ b/src/ncbuf.c
@@ -726,7 +726,7 @@ struct rand_off {
 static struct rand_off *ncb_generate_rand_off(const struct ncbuf *buf)
 {
struct rand_off *roff;
-   roff = calloc(1, sizeof(struct rand_off));
+   roff = calloc(1, sizeof(*roff));
BUG_ON(!roff);
 
roff->off = rand() % (ncb_size(buf));
diff --git a/src/proto_quic.c b/src/proto_quic.c
index 55aa4b50f..ab1bef18f 100644
--- a/src/proto_quic.c
+++ b/src/proto_quic.c
@@ -703,7 +703,7 @@ static int quic_alloc_dghdlrs(void)
 {
int i;
 
-   quic_dghdlrs = calloc(global.nbthread, sizeof(struct quic_dghdlr));
+   quic_dghdlrs = calloc(global.nbthread, sizeof(*quic_dghdlrs));
if (!quic_dghdlrs) {
ha_alert("Failed to allocate the quic datagram handlers.\n");
return 0;
diff --git a/src/quic_sock.c b/src/quic_sock.c
index 6207af703..a391006af 100644
--- a/src/quic_sock.c
+++ b/src/quic_sock.c
@@ -466,7 +466,8 @@ static int quic_alloc_accept_queues(void)
 {
int i;
 
-   quic_accept_queues = calloc(global.nbthread, sizeof(struct 
quic_accept_queue));
+   quic_accept_queues = calloc(global.nbthread,
+   sizeof(*quic_accept_queues));
if (!quic_accept_queues) {
ha_alert("Failed to allocate the quic accept queues.\n");
return 0;
-- 
2.36.1

Re: [PATCH] CLEANUP: tools: Clean up non-QUIC error message handling in str2sa_range()

[Willy Tarreau]

On Sun, May 22, 2022 at 12:40:58PM +0200, Tim Duesterhus wrote:
> If QUIC support is enabled both branches of the ternary conditional are
> identical, upsetting Coverity. Move the full conditional into the non-QUIC
> preprocessor branch to make the code more clear.
> 
> This resolves GitHub issue #1710.

OK that's fine like this. Now merged, thank you Tim!
Willy

[PATCH] CLEANUP: tools: Clean up non-QUIC error message handling in str2sa_range()

[Tim Duesterhus]

If QUIC support is enabled both branches of the ternary conditional are
identical, upsetting Coverity. Move the full conditional into the non-QUIC
preprocessor branch to make the code more clear.

This resolves GitHub issue #1710.
---
 src/tools.c | 7 ---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/src/tools.c b/src/tools.c
index 9e629e5bd..4c93e1e82 100644
--- a/src/tools.c
+++ b/src/tools.c
@@ -1305,13 +1305,14 @@ struct sockaddr_storage *str2sa_range(const char *str, 
int *port, int *low, int
  (proto_type == PROTO_TYPE_DGRAM) ? "datagram" 
: "stream",
  ss.ss_family,
  str,
- (ctrl_type == SOCK_STREAM && proto_type == 
PROTO_TYPE_DGRAM) ?
 #ifndef USE_QUIC
- "; QUIC is not compiled in if this is what 
you were looking for."
+ (ctrl_type == SOCK_STREAM && proto_type == 
PROTO_TYPE_DGRAM)
+ ? "; QUIC is not compiled in if this is what 
you were looking for."
+ : ""
 #else
  ""
 #endif
- :"");
+   );
goto out;
}
 
-- 
2.36.1

Re: [PATCH] CLEANUP: http_ana: Make use of the return value of stream_generate_unique_id()

[Willy Tarreau]

On Wed, May 18, 2022 at 12:22:15AM +0200, Tim Duesterhus wrote:
> Even if `unique_id` and `s->unique_id` are identical it is a bit odd to
> `isttest()` `unique_id` and then use `s->unique_id` in the call to 
> `http_add_header()`.

Agreed, better be consistent. Now applied, thank you Tim!
Willy

[PATCH] CLEANUP: http_ana: Make use of the return value of stream_generate_unique_id()

[Tim Duesterhus]

Even if `unique_id` and `s->unique_id` are identical it is a bit odd to
`isttest()` `unique_id` and then use `s->unique_id` in the call to 
`http_add_header()`.

This "issue" was introduced in a17e66289c08a5bfadc1bb5b5f2c618c9299fe1b,
because before that commit the function returned the length of the ID, as it
was not an ist.
---
 src/http_ana.c | 2 +-
 src/stream.c   | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/http_ana.c b/src/http_ana.c
index 4b3113e2d..69340656a 100644
--- a/src/http_ana.c
+++ b/src/http_ana.c
@@ -646,7 +646,7 @@ int http_process_request(struct stream *s, struct channel 
*req, int an_bit)
 
/* send unique ID if a "unique-id-header" is defined */
if (isttest(sess->fe->header_unique_id) &&
-   unlikely(!http_add_header(htx, sess->fe->header_unique_id, 
s->unique_id)))
+   unlikely(!http_add_header(htx, sess->fe->header_unique_id, 
unique_id)))
goto return_int_err;
}
 
diff --git a/src/stream.c b/src/stream.c
index c7002366f..a02ea9871 100644
--- a/src/stream.c
+++ b/src/stream.c
@@ -2861,7 +2861,7 @@ INITCALL0(STG_INIT, init_stream);
 
 /* Generates a unique ID based on the given , stores it in the given 
 and
  * returns the unique ID.
-
+ *
  * If this function fails to allocate memory IST_NULL is returned.
  *
  * If an ID is already stored within the stream nothing happens existing 
unique ID is
-- 
2.36.1

Re: [PATCH] CLEANUP: Destroy `http_err_chunks` members during deinit

[Willy Tarreau]

Hi Tim,

On Tue, Apr 26, 2022 at 11:35:07PM +0200, Tim Duesterhus wrote:
> To make the deinit function a proper inverse of the init function we need to
> free the `http_err_chunks`:
> 
> ==252081== 311,296 bytes in 19 blocks are still reachable in loss record 
> 50 of 50
> ==252081==at 0x483B7F3: malloc (in 
> /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so)
> ==252081==by 0x2727EE: http_str_to_htx (http_htx.c:914)
> ==252081==by 0x272E60: http_htx_init (http_htx.c:1059)
> ==252081==by 0x26AC87: check_config_validity (cfgparse.c:4170)
> ==252081==by 0x155DFE: init (haproxy.c:2120)
> ==252081==by 0x155DFE: main (haproxy.c:3037)

Indeed. At first I was worried that there could be static buffers in
use there like in the past, but no, that's indeed always initialized
and allocated by http_str_to_htx() so that's both safe and needed.

Applied, thank you!
Willy

[PATCH] CLEANUP: Destroy `http_err_chunks` members during deinit

[Tim Duesterhus]

To make the deinit function a proper inverse of the init function we need to
free the `http_err_chunks`:

==252081== 311,296 bytes in 19 blocks are still reachable in loss record 50 
of 50
==252081==at 0x483B7F3: malloc (in 
/usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so)
==252081==by 0x2727EE: http_str_to_htx (http_htx.c:914)
==252081==by 0x272E60: http_htx_init (http_htx.c:1059)
==252081==by 0x26AC87: check_config_validity (cfgparse.c:4170)
==252081==by 0x155DFE: init (haproxy.c:2120)
==252081==by 0x155DFE: main (haproxy.c:3037)
---
 src/http_htx.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/src/http_htx.c b/src/http_htx.c
index d9584abae..ea4c25f1a 100644
--- a/src/http_htx.c
+++ b/src/http_htx.c
@@ -1112,6 +1112,9 @@ static void http_htx_deinit(void)
LIST_DELETE(_rep->list);
release_http_reply(http_rep);
}
+
+   for (rc = 0; rc < HTTP_ERR_SIZE; rc++)
+   chunk_destroy(_err_chunks[rc]);
 }
 
 REGISTER_CONFIG_POSTPARSER("http_htx", http_htx_init);
-- 
2.36.0

[PATCH] CLEANUP: Wrap `accept4_broken = 1` into additional parenthesis

[Tim Duesterhus]

This makes it clear to static analysis tools that this assignment is
intentional and not a mistyped comparison.
---
 src/sock.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/sock.c b/src/sock.c
index e3d4a6e4c..f11c5b0c4 100644
--- a/src/sock.c
+++ b/src/sock.c
@@ -74,7 +74,7 @@ struct connection *sock_accept_conn(struct listener *l, int 
*status)
(((cfd = accept4(l->rx.fd, (struct sockaddr*)addr, ,
 SOCK_NONBLOCK | (master ? SOCK_CLOEXEC : 0))) == 
-1) &&
 (errno == ENOSYS || errno == EINVAL || errno == EBADF) &&
-(accept4_broken = 1)))
+((accept4_broken = 1
 #endif
{
laddr = sizeof(*conn->src);
-- 
2.34.0

Re: [PATCH] CLEANUP: slz: Mark `reset_refs` as static

[Tim Düsterhus]

Willy,

On 11/5/21 4:43 PM, Willy Tarreau wrote:

Yes, sorry, but I will really deal with that stuff *after* we're done
with haproxy's release.


Yes, absolutely.

I just came across this point on my list while I was double-checking 
whether I had anything important / any loose ends for 2.5 and thought 
I'd sent an email so that I don't forget about this myself :-)


Best regards
Tim Düsterhus

Re: [PATCH] CLEANUP: slz: Mark `reset_refs` as static

[Willy Tarreau]

On Fri, Nov 05, 2021 at 02:17:51PM +0100, Tim Düsterhus wrote:
> Willy,
> 
> On 10/11/21 5:15 PM, Tim Düsterhus wrote:
> > > > > > please also apply to https://github.com/wtarreau/libslz/.
> > > > > > [...]
> > > > > 
> > > > > Now applied, thanks!
> > > > 
> > > > Not seeing anything in the libslz repository yet. Did you forget to 
> > > > push?
> > > > :-)
> > > 
> > > No, I've applied to the haproxy copy only for now, will do slz later,
> > > and after double-checking that no legacy code still uses the function
> > > (becaure in the very first incarnation it had to be called from the
> > > main code, which was not very handy). But I guess that any such code
> > > I could find would just be test code.
> > 
> > Reminder in case you forgot about this and did not just get not around
> > to it.
> > 
> 
> Another reminder, because you did not reply to my previous.

Yes, sorry, but I will really deal with that stuff *after* we're done
with haproxy's release.

Thanks,
Willy

Re: [PATCH] CLEANUP: slz: Mark `reset_refs` as static

[Tim Düsterhus]

Willy,

On 10/11/21 5:15 PM, Tim Düsterhus wrote:

please also apply to https://github.com/wtarreau/libslz/.
[...]


Now applied, thanks!


Not seeing anything in the libslz repository yet. Did you forget to push?
:-)


No, I've applied to the haproxy copy only for now, will do slz later,
and after double-checking that no legacy code still uses the function
(becaure in the very first incarnation it had to be called from the
main code, which was not very handy). But I guess that any such code
I could find would just be test code.


Reminder in case you forgot about this and did not just get not around
to it.



Another reminder, because you did not reply to my previous.

Best regards
Tim Düsterhus

Re: [PATCH] CLEANUP: halog: Remove dead stores

[Willy Tarreau]

Hi Tim,

On Thu, Nov 04, 2021 at 09:04:24PM +0100, Tim Duesterhus wrote:
> Found using clang's scan-build.
(...)
This and your 4 other cleanup patches applied now, thank you!
Willy

[PATCH] CLEANUP: halog: Remove dead stores

[Tim Duesterhus]

Found using clang's scan-build.
---
 admin/halog/halog.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/admin/halog/halog.c b/admin/halog/halog.c
index 900cf5d46..f368c1c6f 100644
--- a/admin/halog/halog.c
+++ b/admin/halog/halog.c
@@ -551,7 +551,8 @@ int convert_date_to_timestamp(const char *field)
d = mo = y = h = m = s = 0;
e = field;
 
-   c = *(e++); // remove '['
+   e++; // remove '['
+
/* day + '/' */
while (1) {
c = *(e++) - '0';
@@ -1148,13 +1149,12 @@ int main(int argc, char **argv)
/* sort all timers */
for (f = 0; f < 5; f++) {
struct eb32_node *n;
-   int val;
 
-   val = 0;
n = eb32_first([f]);
while (n) {
int i;
double d;
+   int val;
 
t = container_of(n, struct timer, node);
last = n->key;
-- 
2.33.1

Re: [EXTERNAL] Re: [PATCH] CLEANUP: http_fetch: Use ist helpers in smp_fetch_http_auth_bearer()

[Willy Tarreau]

On Fri, Oct 29, 2021 at 05:20:23PM +0200, Remi Tricot-Le Breton wrote:
> > > Rémi, am I missing something or is it just that this code snippet indeed
> > > has a bug that was not spotted by the regtests (which I'm fine with,
> > > they're regression tests, not unit tests seeking 100% coverage) ?
> > 
> > You are not missing anything. This is indeed broken, because no reg-test
> > covers calling the `http_auth_bearer` fetch with a string parameter.
> > 
> > Thus the tests always go into the 'else' part where `get_http_auth()`
> > handles the parsing.
> > 
> > The correct fix would then of course be that the parsing logic is moved
> > into a dedicated function (ideally based on the ist API) and not copied
> > into several different functions.
> > 
> > I retract my patch.
> > 
> > Remi: Shall I file an issue to track the duplicated parsing logic or
> > will you handle this based on this ML thread?
> 
> Willy and I talked about this together but I forgot to add the list when I
> first replied to him. Some patches are waiting for a review already,
> including yours.

And I've now applied your v3, which passes the regtests correctly this
time, thanks!

Willy

Re: [EXTERNAL] Re: [PATCH] CLEANUP: http_fetch: Use ist helpers in smp_fetch_http_auth_bearer()

[Remi Tricot-Le Breton]

Hello Tim,

On 29/10/2021 16:57, Tim Düsterhus wrote:

Willy,

On 10/29/21 8:50 AM, Willy Tarreau wrote:

I don't see how this can ever match:

   - we search for a space in the  first characters starting at 


   - if we find one such space, we check if these characters are exactly
 equal to the string "Bearer" (modulo the case), and if so we 
take the

 value.
   => by definition this cannot match since there is no space in 
"Bearer"


It made me doubt about strncasecmp() to the point that I had to write 
some

code to verify I wasn't mistaken about how it worked.

Rémi, am I missing something or is it just that this code snippet indeed
has a bug that was not spotted by the regtests (which I'm fine with,
they're regression tests, not unit tests seeking 100% coverage) ?


You are not missing anything. This is indeed broken, because no 
reg-test covers calling the `http_auth_bearer` fetch with a string 
parameter.


Thus the tests always go into the 'else' part where `get_http_auth()` 
handles the parsing.


The correct fix would then of course be that the parsing logic is 
moved into a dedicated function (ideally based on the ist API) and not 
copied into several different functions.


I retract my patch.

Remi: Shall I file an issue to track the duplicated parsing logic or 
will you handle this based on this ML thread?


Willy and I talked about this together but I forgot to add the list when 
I first replied to him. Some patches are waiting for a review already, 
including yours.


Rémi

Re: [PATCH] CLEANUP: http_fetch: Use ist helpers in smp_fetch_http_auth_bearer()

[Tim Düsterhus]

Willy,

On 10/29/21 8:50 AM, Willy Tarreau wrote:

I don't see how this can ever match:

   - we search for a space in the  first characters starting at 
   - if we find one such space, we check if these characters are exactly
 equal to the string "Bearer" (modulo the case), and if so we take the
 value.
   => by definition this cannot match since there is no space in "Bearer"

It made me doubt about strncasecmp() to the point that I had to write some
code to verify I wasn't mistaken about how it worked.

Rémi, am I missing something or is it just that this code snippet indeed
has a bug that was not spotted by the regtests (which I'm fine with,
they're regression tests, not unit tests seeking 100% coverage) ?


You are not missing anything. This is indeed broken, because no reg-test 
covers calling the `http_auth_bearer` fetch with a string parameter.


Thus the tests always go into the 'else' part where `get_http_auth()` 
handles the parsing.


The correct fix would then of course be that the parsing logic is moved 
into a dedicated function (ideally based on the ist API) and not copied 
into several different functions.


I retract my patch.

Remi: Shall I file an issue to track the duplicated parsing logic or 
will you handle this based on this ML thread?


Best regards
Tim Düsterhus

Re: [PATCH] CLEANUP: http_fetch: Use ist helpers in smp_fetch_http_auth_bearer()

[Willy Tarreau]

On Thu, Oct 14, 2021 at 07:48:08PM +0200, Tim Duesterhus wrote:
> Remi,
> 
> please find a suggested cleanup for your JWT patch series. I think that
> using the ist functions results in easier to understand code, because you
> don't need to manually calculate lengths and offsets.
> 
> Apply with `git am --scissors` to automatically cut the commit message.
> 
> -- >8 --
> Using the ist helper functions is arguably cleaner than using C's regular
> string functions on an ist.

It's indeed more readable but I must confess I'm now confused by both
versions regarding what they're trying to do:

>   ctx.blk = NULL;
>   if (http_find_header(htx, hdr_name, , 0)) {
> - char *space = NULL;
> - space = memchr(ctx.value.ptr, ' ', ctx.value.len);
> - if (space && strncasecmp("Bearer", ctx.value.ptr, 
> ctx.value.len) == 0) {
> - chunk_initlen(_val, space+1, 0, ctx.value.len - 
> (space - ctx.value.ptr) - 1);

I don't see how this can ever match:

  - we search for a space in the  first characters starting at 
  - if we find one such space, we check if these characters are exactly
equal to the string "Bearer" (modulo the case), and if so we take the
value.
  => by definition this cannot match since there is no space in "Bearer"

It made me doubt about strncasecmp() to the point that I had to write some
code to verify I wasn't mistaken about how it worked.

Rémi, am I missing something or is it just that this code snippet indeed
has a bug that was not spotted by the regtests (which I'm fine with,
they're regression tests, not unit tests seeking 100% coverage) ?

> + struct ist type = istsplit(, ' ');
> +
> + if (isteqi(type, ist("Bearer")) && istend(type) != 
> istend(ctx.value)) {
> + chunk_initlen(_val, istptr(ctx.value), 0, 
> istlen(ctx.value));

Here I would find it more understandable to test like this:

if (isteqi(type, ist("Bearer")) && istlen(ctx.value)) {

Also, I had a check to RFC2617 and there's *at least* one space after
the auth scheme, so we need to skip all of them. For me that would
give this:

struct ist type = istsplit(, ' ');

ctx = istskip(ctx, ' ');
if (isteqi(type, ist("Bearer")) && istlen(ctx.value))
chunk_initlen(_val, istptr(ctx.value), 0, 
istlen(ctx.value));

But I'd like someone to double-check all this, and possibly submit a
fix if anything needs to be fixed (at least the multiple spaces case
seems to require one).

Thanks,
Willy

Re: [PATCH] CLEANUP: http_fetch: Use ist helpers in smp_fetch_http_auth_bearer()

[Tim Düsterhus]

Willy,

On 10/14/21 7:48 PM, Tim Duesterhus wrote:

please find a suggested cleanup for your JWT patch series. I think that
using the ist functions results in easier to understand code, because you
don't need to manually calculate lengths and offsets.



Here's another patch that did not receive any attention. I originally 
sent it to Remi (and the list) only.


Best regards
Tim Düsterhus

[PATCH] CLEANUP: hlua: Remove obsolete branch in `hlua_alloc()`

[Tim Duesterhus]

This branch is no longer required, because the `!nsize` case is handled for any
value of `ptr` now.

see 22586524e32f14c44239063088a38ccea8abc9b7
see a5efdff93c36f75345a2a18f18bffee9b602bc7b
---
 src/hlua.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/src/hlua.c b/src/hlua.c
index ac61a3171..0e12614af 100644
--- a/src/hlua.c
+++ b/src/hlua.c
@@ -11463,9 +11463,6 @@ static void *hlua_alloc(void *ud, void *ptr, size_t 
osize, size_t nsize)
struct hlua_mem_allocator *zone = ud;
size_t limit, old, new;
 
-   if (unlikely(!ptr && !nsize))
-   return NULL;
-
/* a limit of ~0 means unlimited and boot complete, so there's no need
 * for accounting anymore.
 */
-- 
2.33.1

Re: [PATCH] CLEANUP: Consistently `unsigned int` for bitfields

[Tim Düsterhus]

Willy,

On 10/18/21 10:51 AM, Willy Tarreau wrote:

On Mon, Oct 18, 2021 at 09:18:12AM +0200, Tim Düsterhus wrote:

Hu, interesting. Is the GitHub Mirror Sync broken? I'm seeing changes in
https://git.haproxy.org/?p=haproxy.git, but not in GitHub.


So it was in relation with the Painful Access Token apparently. The
mirror user was not allowed anymore to push to .github/workflows:

   $ git push github
   Counting objects: 99, done.
   Delta compression using up to 2 threads.
   Compressing objects: 100% (63/63), done.
   Writing objects: 100% (63/63), 6.69 KiB | 0 bytes/s, done.
   Total 63 (delta 51), reused 0 (delta 0)
   remote: Resolving deltas: 100% (51/51), completed with 34 local objects.
   To https://github.com/haproxy/haproxy.git
! [remote rejected] master -> master (refusing to allow a Personal Access 
Token to create or update workflow `.github/workflows/codespell.yml` without 
`workflow` scope)

I don't really see the relation with any of the recent changes. Thus I
switched to SSH and got rid of the stupid clear-text PAT and now it's
OK again.



GitHub Action Workflows are pretty powerful and can do all kinds of 
stuff within a repository. I assume that GitHub wanted to increase the 
security by not allowing arbitrary tokens to change them, when the 
purpose of the token is something entirely different.


I assume that simply using SSH will be stable going forward. That's all 
I ever used for write access since I signed up to GitHub.


Best regards
Tim Düsterhus

Re: [PATCH] CLEANUP: Consistently `unsigned int` for bitfields

[Willy Tarreau]

On Mon, Oct 18, 2021 at 09:18:12AM +0200, Tim Düsterhus wrote:
> Hu, interesting. Is the GitHub Mirror Sync broken? I'm seeing changes in
> https://git.haproxy.org/?p=haproxy.git, but not in GitHub.

So it was in relation with the Painful Access Token apparently. The
mirror user was not allowed anymore to push to .github/workflows:

  $ git push github
  Counting objects: 99, done.
  Delta compression using up to 2 threads.
  Compressing objects: 100% (63/63), done.
  Writing objects: 100% (63/63), 6.69 KiB | 0 bytes/s, done.
  Total 63 (delta 51), reused 0 (delta 0)
  remote: Resolving deltas: 100% (51/51), completed with 34 local objects.
  To https://github.com/haproxy/haproxy.git
   ! [remote rejected] master -> master (refusing to allow a Personal Access 
Token to create or update workflow `.github/workflows/codespell.yml` without 
`workflow` scope)

I don't really see the relation with any of the recent changes. Thus I
switched to SSH and got rid of the stupid clear-text PAT and now it's
OK again.

Thanks,
Willy

Re: [PATCH] CLEANUP: Consistently `unsigned int` for bitfields

[Willy Tarreau]

On Mon, Oct 18, 2021 at 09:18:12AM +0200, Tim Düsterhus wrote:
> Willy,
> 
> On 10/18/21 9:15 AM, Willy Tarreau wrote:
> > On Mon, Oct 18, 2021 at 09:09:01AM +0200, Tim Düsterhus wrote:
> > > Feel free to replace 'unsigned int' with 'uint' and reformat the struct as
> > > needed.
> > 
> > Done an pushed, thank you!
> > Willy
> > 
> 
> Hu, interesting. Is the GitHub Mirror Sync broken? I'm seeing changes in
> https://git.haproxy.org/?p=haproxy.git, but not in GitHub.

Indeed, it seems that nothing was updated since dev10 this week-end.
I'll have a look.

Thanks for reporting it!
Willy

Re: [PATCH] CLEANUP: Consistently `unsigned int` for bitfields

[Tim Düsterhus]

Willy,

On 10/18/21 9:15 AM, Willy Tarreau wrote:

On Mon, Oct 18, 2021 at 09:09:01AM +0200, Tim Düsterhus wrote:

Feel free to replace 'unsigned int' with 'uint' and reformat the struct as
needed.


Done an pushed, thank you!
Willy



Hu, interesting. Is the GitHub Mirror Sync broken? I'm seeing changes in 
https://git.haproxy.org/?p=haproxy.git, but not in GitHub.


Best regards
Tim Düsterhus

Re: [PATCH] CLEANUP: Consistently `unsigned int` for bitfields

[Willy Tarreau]

On Mon, Oct 18, 2021 at 09:09:01AM +0200, Tim Düsterhus wrote:
> Feel free to replace 'unsigned int' with 'uint' and reformat the struct as
> needed.

Done an pushed, thank you!
Willy

Re: [PATCH] CLEANUP: Consistently `unsigned int` for bitfields

[Tim Düsterhus]

Willy,

On 10/18/21 7:22 AM, Willy Tarreau wrote:

On Sat, Oct 16, 2021 at 06:24:18PM +0200, Tim Duesterhus wrote:

see 6a0dd733906611dea958cf74b9f51bb16028ae20

Found using GitHub's CodeQL scan.
---
  include/haproxy/stick_table-t.h | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/haproxy/stick_table-t.h b/include/haproxy/stick_table-t.h
index 3b1f2b3ef..133f992b5 100644
--- a/include/haproxy/stick_table-t.h
+++ b/include/haproxy/stick_table-t.h
@@ -125,8 +125,8 @@ struct stktable_data_type {
const char *name; /* name of the data type */
int std_type; /* standard type we can use for this data, STD_T_* */
int arg_type; /* type of optional argument, ARG_T_* */
-   int is_array:1;   /* this is an array of gpc/gpt */
-   int is_local:1;   /* this is local only and never learned */
+   unsigned int is_array:1;   /* this is an array of gpc/gpt */
+   unsigned int is_local:1;   /* this is local only and never learned */


Please note, since last patch we've added shorter type names like "uint"
for the unsigned int, that help preserve alignment. Do you mind if I adapt
your patch ?


Feel free to replace 'unsigned int' with 'uint' and reformat the struct 
as needed.


Best regards
Tim Düsterhus

Re: [PATCH] CLEANUP: assorted typo fixes in the code and comments

[Willy Tarreau]

On Fri, Oct 15, 2021 at 04:18:21PM +0500, Ilya Shipitsin wrote:
> This is 27th iteration of typo fixes

Merged, thanks Ilya!
Willy

Re: [PATCH] CLEANUP: Consistently `unsigned int` for bitfields

[Willy Tarreau]

On Sat, Oct 16, 2021 at 06:24:18PM +0200, Tim Duesterhus wrote:
> see 6a0dd733906611dea958cf74b9f51bb16028ae20
> 
> Found using GitHub's CodeQL scan.
> ---
>  include/haproxy/stick_table-t.h | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/include/haproxy/stick_table-t.h b/include/haproxy/stick_table-t.h
> index 3b1f2b3ef..133f992b5 100644
> --- a/include/haproxy/stick_table-t.h
> +++ b/include/haproxy/stick_table-t.h
> @@ -125,8 +125,8 @@ struct stktable_data_type {
>   const char *name; /* name of the data type */
>   int std_type; /* standard type we can use for this data, STD_T_* */
>   int arg_type; /* type of optional argument, ARG_T_* */
> - int is_array:1;   /* this is an array of gpc/gpt */
> - int is_local:1;   /* this is local only and never learned */
> + unsigned int is_array:1;   /* this is an array of gpc/gpt */
> + unsigned int is_local:1;   /* this is local only and never learned */

Please note, since last patch we've added shorter type names like "uint"
for the unsigned int, that help preserve alignment. Do you mind if I adapt
your patch ?

Willy

[PATCH] CLEANUP: Consistently `unsigned int` for bitfields

[Tim Duesterhus]

see 6a0dd733906611dea958cf74b9f51bb16028ae20

Found using GitHub's CodeQL scan.
---
 include/haproxy/stick_table-t.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/haproxy/stick_table-t.h b/include/haproxy/stick_table-t.h
index 3b1f2b3ef..133f992b5 100644
--- a/include/haproxy/stick_table-t.h
+++ b/include/haproxy/stick_table-t.h
@@ -125,8 +125,8 @@ struct stktable_data_type {
const char *name; /* name of the data type */
int std_type; /* standard type we can use for this data, STD_T_* */
int arg_type; /* type of optional argument, ARG_T_* */
-   int is_array:1;   /* this is an array of gpc/gpt */
-   int is_local:1;   /* this is local only and never learned */
+   unsigned int is_array:1;   /* this is an array of gpc/gpt */
+   unsigned int is_local:1;   /* this is local only and never learned */
 };
 
 /* stick table keyword type */
-- 
2.33.0

[PATCH] CLEANUP: assorted typo fixes in the code and comments

[Ilya Shipitsin]

This is 27th iteration of typo fixes
---
 doc/configuration.txt|  2 +-
 include/haproxy/h3.h |  2 +-
 include/haproxy/mux_quic-t.h |  2 +-
 include/haproxy/mux_quic.h   |  2 +-
 include/haproxy/qpack-t.h|  2 +-
 reg-tests/http-messaging/http_request_buffer.vtc |  4 ++--
 reg-tests/ssl/ssl_errors.vtc |  2 +-
 src/h3.c |  4 ++--
 src/hlua.c   |  2 +-
 src/http_client.c|  6 +++---
 src/xprt_quic.c  | 12 ++--
 11 files changed, 20 insertions(+), 20 deletions(-)

diff --git a/doc/configuration.txt b/doc/configuration.txt
index c242698c4..5774bf7f4 100644
--- a/doc/configuration.txt
+++ b/doc/configuration.txt
@@ -21273,7 +21273,7 @@ Detailed fields description :
   - "ssl_fc_err" is the last error of the first SSL error stack that was
 raised on the connection from the frontend's perspective. It might be used
 to detect SSL handshake errors for instance. It will be 0 if everything
-went well. See the "ssl_fc_err" sample fetch's decription for more
+went well. See the "ssl_fc_err" sample fetch's description for more
 information.
 
   - "ssl_c_err" is the status of the client's certificate verification process.
diff --git a/include/haproxy/h3.h b/include/haproxy/h3.h
index 4c329b8ab..e61a2d9b2 100644
--- a/include/haproxy/h3.h
+++ b/include/haproxy/h3.h
@@ -1,6 +1,6 @@
 /*
  * include/haproxy/h3.h
- * This file containts types for H3
+ * This file contains types for H3
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Lesser General Public
diff --git a/include/haproxy/mux_quic-t.h b/include/haproxy/mux_quic-t.h
index 09e0585f6..b93a8a578 100644
--- a/include/haproxy/mux_quic-t.h
+++ b/include/haproxy/mux_quic-t.h
@@ -1,6 +1,6 @@
 /*
  * include/haproxy/mux_quic-t.h
- * This file containts types for QUIC mux-demux.
+ * This file contains types for QUIC mux-demux.
  *
  * Copyright 2021 HAProxy Technologies, Frédéric Lécaille 

  *
diff --git a/include/haproxy/mux_quic.h b/include/haproxy/mux_quic.h
index 2991e871a..332c5291a 100644
--- a/include/haproxy/mux_quic.h
+++ b/include/haproxy/mux_quic.h
@@ -1,6 +1,6 @@
 /*
  * include/haproxy/mux_quic-t.h
- * This file containts prototypes for QUIC mux-demux.
+ * This file contains prototypes for QUIC mux-demux.
  *
  * Copyright 2021 HAProxy Technologies, Frédéric Lécaille 

  *
diff --git a/include/haproxy/qpack-t.h b/include/haproxy/qpack-t.h
index 832094e8e..1200eef94 100644
--- a/include/haproxy/qpack-t.h
+++ b/include/haproxy/qpack-t.h
@@ -1,6 +1,6 @@
 /*
  * include/haproxy/qpack-t.h
- * This file containts types for QPACK
+ * This file contains types for QPACK
  *
  * Copyright 2021 HAProxy Technologies, Frédéric Lécaille 

  *
diff --git a/reg-tests/http-messaging/http_request_buffer.vtc 
b/reg-tests/http-messaging/http_request_buffer.vtc
index 8ed683be7..c41781425 100644
--- a/reg-tests/http-messaging/http_request_buffer.vtc
+++ b/reg-tests/http-messaging/http_request_buffer.vtc
@@ -87,7 +87,7 @@ client c2 -connect ${h1_fe1_sock} {
 } -run
 
 # Payload is fully sent in 2 steps (with a small delay, smaller than the client
-# timeout) and splitted on a chunk size.
+# timeout) and split on a chunk size.
 #   ==> Request must be sent to the server. A 200 must be received
 client c3 -connect ${h1_fe1_sock} {
send "POST /1  HTTP/1.1\r\nTransfer-Encoding: 
chunked\r\n\r\n1\r\n1\r\n1"
@@ -98,7 +98,7 @@ client c3 -connect ${h1_fe1_sock} {
 } -run
 
 # Last CRLF of the request payload is missing but payload is sent in 2 steps
-# (with a small delay, smaller than the client timeout) and splitted on a chunk
+# (with a small delay, smaller than the client timeout) and split on a chunk
 # size. The client aborts before sending the last CRLF.
 #   ==> Request must be handled as an error with 'CR--' termination state.
 client c4 -connect ${h1_fe1_sock} {
diff --git a/reg-tests/ssl/ssl_errors.vtc b/reg-tests/ssl/ssl_errors.vtc
index 7daf2102f..ef83e3e60 100644
--- a/reg-tests/ssl/ssl_errors.vtc
+++ b/reg-tests/ssl/ssl_errors.vtc
@@ -54,7 +54,7 @@ syslog Slg_cust_fmt -level info {
 
 barrier b1 sync
 
-# In case of an error occuring before the certificate verification process,
+# In case of an error occurring before the certificate verification 
process,
 # the client certificate chain is never parsed and verified so we can't
 # have information about the client's certificate.
 recv
diff --git a/src/h3.c b/src/h3.c
index fc68432f8..cd8f32d55 100644
--- a/src/h3.c
+++ b/src/h3.c
@@ -196,7 +196,7 @@ static int h3_decode_qcs(struct qcs *qcs, void *ctx)
cs->ctx = qcs;
stream_create_from_cs(cs, _buf);
 
-   /* buffer is 

[PATCH] CLEANUP: http_fetch: Use ist helpers in smp_fetch_http_auth_bearer()

[Tim Duesterhus]

Remi,

please find a suggested cleanup for your JWT patch series. I think that
using the ist functions results in easier to understand code, because you
don't need to manually calculate lengths and offsets.

Apply with `git am --scissors` to automatically cut the commit message.

-- >8 --
Using the ist helper functions is arguably cleaner than using C's regular
string functions on an ist.
---
 src/http_fetch.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/src/http_fetch.c b/src/http_fetch.c
index 7b4e41d85..61545d66f 100644
--- a/src/http_fetch.c
+++ b/src/http_fetch.c
@@ -1369,10 +1369,10 @@ static int smp_fetch_http_auth_bearer(const struct arg 
*args, struct sample *smp
 
ctx.blk = NULL;
if (http_find_header(htx, hdr_name, , 0)) {
-   char *space = NULL;
-   space = memchr(ctx.value.ptr, ' ', ctx.value.len);
-   if (space && strncasecmp("Bearer", ctx.value.ptr, 
ctx.value.len) == 0) {
-   chunk_initlen(_val, space+1, 0, 
ctx.value.len - (space - ctx.value.ptr) - 1);
+   struct ist type = istsplit(, ' ');
+
+   if (isteqi(type, ist("Bearer")) && istend(type) != 
istend(ctx.value)) {
+   chunk_initlen(_val, istptr(ctx.value), 
0, istlen(ctx.value));
}
}
}
-- 
2.33.0

Re: [PATCH] CLEANUP: slz: Mark `reset_refs` as static

[Tim Düsterhus]

Willy,

On 9/24/21 3:36 PM, Willy Tarreau wrote:

On Fri, Sep 24, 2021 at 03:33:14PM +0200, Tim Düsterhus wrote:

Willy,

On 9/24/21 3:09 PM, Willy Tarreau wrote:

please also apply to https://github.com/wtarreau/libslz/.
[...]


Now applied, thanks!


Not seeing anything in the libslz repository yet. Did you forget to push?
:-)


No, I've applied to the haproxy copy only for now, will do slz later,
and after double-checking that no legacy code still uses the function
(becaure in the very first incarnation it had to be called from the
main code, which was not very handy). But I guess that any such code
I could find would just be test code.


Reminder in case you forgot about this and did not just get not around 
to it.


Best regards
Tim Düsterhus

Re: [PATCH] CLEANUP: slz: Mark `reset_refs` as static

[Willy Tarreau]

On Fri, Sep 24, 2021 at 03:33:14PM +0200, Tim Düsterhus wrote:
> Willy,
> 
> On 9/24/21 3:09 PM, Willy Tarreau wrote:
> > > please also apply to https://github.com/wtarreau/libslz/.
> > > [...]
> > 
> > Now applied, thanks!
> 
> Not seeing anything in the libslz repository yet. Did you forget to push?
> :-)

No, I've applied to the haproxy copy only for now, will do slz later,
and after double-checking that no legacy code still uses the function
(becaure in the very first incarnation it had to be called from the
main code, which was not very handy). But I guess that any such code
I could find would just be test code.

Cheers,
Willy

Re: [PATCH] CLEANUP: slz: Mark `reset_refs` as static

[Tim Düsterhus]

Willy,

On 9/24/21 3:09 PM, Willy Tarreau wrote:

please also apply to https://github.com/wtarreau/libslz/.
[...]


Now applied, thanks!


Not seeing anything in the libslz repository yet. Did you forget to 
push? :-)


Best regards
Tim Düsterhus

Re: [PATCH] CLEANUP: slz: Mark `reset_refs` as static

[Willy Tarreau]

Hi Tim,

On Mon, Sep 20, 2021 at 07:59:42PM +0200, Tim Duesterhus wrote:
> Willy,
> 
> please also apply to https://github.com/wtarreau/libslz/.
> Best regards
> Tim Düsterhus
> 
> Apply with `git am --scissors` to automatically cut the commit message.
> 
> -- >8 --
> This function has no prototype and is not used outside of slz.c.

Now applied, thanks!
Willy

[PATCH] CLEANUP: slz: Mark `reset_refs` as static

[Tim Duesterhus]

Willy,

please also apply to https://github.com/wtarreau/libslz/.

Best regards
Tim Düsterhus

Apply with `git am --scissors` to automatically cut the commit message.

-- >8 --
This function has no prototype and is not used outside of slz.c.
---
 src/slz.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/slz.c b/src/slz.c
index 9aacb9b7a..23912da6a 100644
--- a/src/slz.c
+++ b/src/slz.c
@@ -466,7 +466,7 @@ static inline long memmatch(const unsigned char *a, const 
unsigned char *b, long
  * be applied to 64-bit aligned data exclusively, which makes it slightly
  * faster than the regular memset() since no alignment check is performed.
  */
-void reset_refs(union ref *refs, long count)
+static void reset_refs(union ref *refs, long count)
 {
/* avoid a shift/mask by casting to void* */
union ref *end = (void *)refs + count;
-- 
2.33.0

[PATCH] CLEANUP: Move XXH3 macro from haproxy/compat.h to haproxy/xxhash.h

[Tim Duesterhus]

This moves all the xxhash functionality into a single location.

see d5fc8fcb86eb99831626051b3055bea7ca93a074
---
 include/haproxy/compat.h | 7 ---
 include/haproxy/xxhash.h | 7 +++
 2 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/include/haproxy/compat.h b/include/haproxy/compat.h
index 72dc3dc4f..886b7a365 100644
--- a/include/haproxy/compat.h
+++ b/include/haproxy/compat.h
@@ -288,13 +288,6 @@ typedef struct { } empty_t;
  */
 #define MAX_SEND_FD 253
 
-/* Make the new complex name for the xxhash function easier to remember
- * and use.
- */
-#ifndef XXH3
-#define XXH3(data, len, seed) XXH3_64bits_withSeed(data, len, seed)
-#endif
-
 #endif /* _HAPROXY_COMPAT_H */
 
 /*
diff --git a/include/haproxy/xxhash.h b/include/haproxy/xxhash.h
index 83a2fb71c..cd333e645 100644
--- a/include/haproxy/xxhash.h
+++ b/include/haproxy/xxhash.h
@@ -42,4 +42,11 @@
 
 #include 
 
+/* Make the new complex name for the xxhash function easier to remember
+ * and use.
+ */
+#ifndef XXH3
+#define XXH3(data, len, seed) XXH3_64bits_withSeed(data, len, seed)
+#endif
+
 #endif
-- 
2.33.0

Re: [PATCH] CLEANUP: Move XXH3 macro from haproxy/compat.h to haproxy/xxhash.h

[Willy Tarreau]

On Sat, Sep 11, 2021 at 08:29:46PM +0200, Tim Duesterhus wrote:
> This moves all the xxhash functionality into a single location.

Wow that was fast, now merged, thanks!
Willy

Re: [PATCH] CLEANUP: Add haproxy/xxhash.h to avoid modifying import/xxhash.h

[Willy Tarreau]

On Sat, Sep 11, 2021 at 08:06:01PM +0200, Dragan Dosen wrote:
> Hi Tim,
> 
> On 11. 09. 2021. 17:51, Tim Duesterhus wrote:
> > This solves setting XXH_INLINE_ALL in a cleaner way, because the imported
> > header is not modified, easing future updates.
> > 
> > see 6f7cc11e6dd0f01b437fba893da2edd2362660a2
> > ---
> > (...)
> 
> Thanks!
> 
> We might also want to move the "XXH3" defined in include/haproxy/compat.h to
> include/haproxy/xxhash.h; see 967e7e79a ("MEDIUM: xxhash: use the XXH3
> functions to generate 64-bit hashes"). Just to have all of our xxHash
> related defines in a single file.

That would indeed be cleaner now that there's a dedicated file.

Willy

Re: [PATCH] CLEANUP: Add haproxy/xxhash.h to avoid modifying import/xxhash.h

[Dragan Dosen]

Hi Tim,

On 11. 09. 2021. 17:51, Tim Duesterhus wrote:

This solves setting XXH_INLINE_ALL in a cleaner way, because the imported
header is not modified, easing future updates.

see 6f7cc11e6dd0f01b437fba893da2edd2362660a2
---
(...)


Thanks!

We might also want to move the "XXH3" defined in 
include/haproxy/compat.h to include/haproxy/xxhash.h; see 967e7e79a 
("MEDIUM: xxhash: use the XXH3 functions to generate 64-bit hashes"). 
Just to have all of our xxHash related defines in a single file.


Best regards,
Dragan Dosen

Re: [PATCH] CLEANUP: Add haproxy/xxhash.h to avoid modifying import/xxhash.h

[Willy Tarreau]

Hi Tim,

On Sat, Sep 11, 2021 at 05:51:13PM +0200, Tim Duesterhus wrote:
> This solves setting XXH_INLINE_ALL in a cleaner way, because the imported
> header is not modified, easing future updates.

Excellent idea, I like this. It's indeed much cleaner and more logical
this way. This may also serve as an example of how to better integrate
external components in the future if needed.

Now applied, thanks!
Willy

[PATCH] CLEANUP: Add haproxy/xxhash.h to avoid modifying import/xxhash.h

[Tim Duesterhus]

This solves setting XXH_INLINE_ALL in a cleaner way, because the imported
header is not modified, easing future updates.

see 6f7cc11e6dd0f01b437fba893da2edd2362660a2
---
 addons/51degrees/51d.c |  2 +-
 admin/dyncookie/dyncookie.c|  3 ++-
 include/haproxy/activity.h |  2 +-
 include/haproxy/connection-t.h |  2 +-
 include/haproxy/defaults.h |  7 --
 include/haproxy/xxhash.h   | 45 ++
 include/import/xxhash.h|  2 --
 src/pattern.c  |  2 +-
 src/sample.c   |  2 +-
 src/server.c   |  3 +--
 src/server_state.c |  2 +-
 src/ssl_sock.c |  2 +-
 12 files changed, 55 insertions(+), 19 deletions(-)
 create mode 100644 include/haproxy/xxhash.h

diff --git a/addons/51degrees/51d.c b/addons/51degrees/51d.c
index 6dfa578e2..5d686953c 100644
--- a/addons/51degrees/51d.c
+++ b/addons/51degrees/51d.c
@@ -1,7 +1,6 @@
 #include 
 
 #include 
-#include 
 #include 
 #include 
 #include 
@@ -15,6 +14,7 @@
 #include 
 #include 
 #include 
+#include 
 #include <51Degrees.h>
 
 struct _51d_property_names {
diff --git a/admin/dyncookie/dyncookie.c b/admin/dyncookie/dyncookie.c
index 0c778eb7a..ddb71a748 100644
--- a/admin/dyncookie/dyncookie.c
+++ b/admin/dyncookie/dyncookie.c
@@ -14,7 +14,8 @@
 #include 
 #include 
 #include 
-#include 
+
+#include 
 
 __attribute__((noreturn)) void die(int code, const char *format, ...)
 {
diff --git a/include/haproxy/activity.h b/include/haproxy/activity.h
index 42569f203..66a5f3be7 100644
--- a/include/haproxy/activity.h
+++ b/include/haproxy/activity.h
@@ -22,11 +22,11 @@
 #ifndef _HAPROXY_ACTIVITY_H
 #define _HAPROXY_ACTIVITY_H
 
-#include 
 #include 
 #include 
 #include 
 #include 
+#include 
 
 extern unsigned int profiling;
 extern unsigned long task_profiling_mask;
diff --git a/include/haproxy/connection-t.h b/include/haproxy/connection-t.h
index ff8927dba..1afae9624 100644
--- a/include/haproxy/connection-t.h
+++ b/include/haproxy/connection-t.h
@@ -30,7 +30,6 @@
 
 #include 
 #include 
-#include 
 
 #include 
 #include 
@@ -38,6 +37,7 @@
 #include 
 #include 
 #include 
+#include 
 
 /* referenced below */
 struct connection;
diff --git a/include/haproxy/defaults.h b/include/haproxy/defaults.h
index 16058817e..19a9cb623 100644
--- a/include/haproxy/defaults.h
+++ b/include/haproxy/defaults.h
@@ -404,13 +404,6 @@
 #define MAX_POLLERS10
 #endif
 
-/* Make all xxhash functions inline, with implementations being directly
- * included within xxhash.h.
- */
-#ifndef XXH_INLINE_ALL
-#define XXH_INLINE_ALL
-#endif
-
 /* system sysfs directory */
 #define NUMA_DETECT_SYSTEM_SYSFS_PATH "/sys/devices/system"
 
diff --git a/include/haproxy/xxhash.h b/include/haproxy/xxhash.h
new file mode 100644
index 0..83a2fb71c
--- /dev/null
+++ b/include/haproxy/xxhash.h
@@ -0,0 +1,45 @@
+/*
+ * Copyright (C) 2020 Dragan Dosen 
+ * Copyright (C) 2021 Tim Duesterhus 
+ *
+ * BSD 2-Clause License (https://www.opensource.org/licenses/bsd-license.php)
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions are
+ * met:
+ *
+ ** Redistributions of source code must retain the above copyright
+ *  notice, this list of conditions and the following disclaimer.
+ ** Redistributions in binary form must reproduce the above
+ *  copyright notice, this list of conditions and the following disclaimer
+ *  in the documentation and/or other materials provided with the
+ *  distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+ * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+ * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+ * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+ * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+ * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+ * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+#ifndef _HAPROXY_XXHASH_H
+#define _HAPROXY_XXHASH_H
+
+/* Make all xxhash functions inline, with implementations being directly
+ * included within xxhash.h.
+ */
+#ifndef XXH_INLINE_ALL
+#define XXH_INLINE_ALL
+#else
+#error "XXH_INLINE_ALL is already defined."
+#endif
+
+#include 
+
+#endif
diff --git a/include/import/xxhash.h b/include/import/xxhash.h
index 24326bc33..d0e3e241e 100644
--- a/include/import/xxhash.h
+++ b/include/import/xxhash.h
@@ -75,8 +75,6 @@ XXH326.8 GB/s6.0 GB/s
 extern "C" {
 #endif
 
-#include 
-
 /* 
  *  

Re: [PATCH] CLEANUP: Add missing include guard to signal.h

[Willy Tarreau]

On Wed, Sep 01, 2021 at 09:23:25PM +0200, Tim Duesterhus wrote:
> Willy,
> 
> this also affects include/import/lru.h, include/import/xxhash.h, and
> include/import/sha1.h. But I did not touch these, as they are within import/*

Yes, good point, I also prefer to really limit the amount of changes
there and only modify them if there is a confirmed problem.

Applied now, thank you!
Willy

[PATCH] CLEANUP: Add missing include guard to signal.h

[Tim Duesterhus]

Willy,

this also affects include/import/lru.h, include/import/xxhash.h, and
include/import/sha1.h. But I did not touch these, as they are within import/*

Best regards
Tim Düsterhus

Apply with `git am --scissors` to automatically cut the commit message.

-- >8 --
Found using GitHub's CodeQL scan.
---
 include/haproxy/signal.h | 5 +
 1 file changed, 5 insertions(+)

diff --git a/include/haproxy/signal.h b/include/haproxy/signal.h
index 763cfb0be..331b7fdc6 100644
--- a/include/haproxy/signal.h
+++ b/include/haproxy/signal.h
@@ -11,6 +11,9 @@
  *
  */
 
+#ifndef _HAPROXY_SIGNAL_H
+#define _HAPROXY_SIGNAL_H
+
 #include 
 
 #include 
@@ -39,6 +42,8 @@ static inline void signal_process_queue()
__signal_process_queue();
 }
 
+#endif /* _HAPROXY_SIGNAL_H */
+
 /*
  * Local variables:
  *  c-indent-level: 8
-- 
2.33.0

Re: [PATCH] CLEANUP: Prevent channel-t.h from being detected as C++ by GitHub

[Tim Düsterhus]

Willy,

On 6/20/21 11:49 AM, Willy Tarreau wrote:

Well, I would say that this heuristic is totally broken because it should at
least ensure there's no more word before a brace on the line. However I agree
that usually we prefer to respect the comment format starting with an asterisk
at the beginning of each line, so I'm OK with the change and merged the patch.


I agree and I certainly would not have proposed it, if it was *just* to 
satisfy this heuristic. With the other reason of matching the comment 
format I considered it acceptable, especially since it did not cause 
rewrapping :-)


Best regards
Tim Düsterhus

Re: [PATCH] CLEANUP: Prevent channel-t.h from being detected as C++ by GitHub

[Willy Tarreau]

Hi Tim,

On Sat, Jun 19, 2021 at 04:56:30PM +0200, Tim Duesterhus wrote:
> GitHub uses github/linguist to determine the programming language used for 
> each
> source file to show statistics and to power the search. In cases of unique 
> file
> extensions this is easy, but for `.h` files the situation is less clear as 
> they
> are used for C, C++, Objective C and more. In these cases linguist makes use 
> of
> heuristics to determine the language.
> 
> One of these heuristics for C++ is that the file contains a line beginning 
> with
> `try`, only preceded by whitespace indentation. This heuristic matches the 
> long
> comment at the bottom of `channel-t.h`, as one sentence includes the word 
> `try`
> after a linebreak.

Well, I would say that this heuristic is totally broken because it should at
least ensure there's no more word before a brace on the line. However I agree
that usually we prefer to respect the comment format starting with an asterisk
at the beginning of each line, so I'm OK with the change and merged the patch.

Thanks!
Willy

[PATCH] CLEANUP: Prevent channel-t.h from being detected as C++ by GitHub

[Tim Duesterhus]

GitHub uses github/linguist to determine the programming language used for each
source file to show statistics and to power the search. In cases of unique file
extensions this is easy, but for `.h` files the situation is less clear as they
are used for C, C++, Objective C and more. In these cases linguist makes use of
heuristics to determine the language.

One of these heuristics for C++ is that the file contains a line beginning with
`try`, only preceded by whitespace indentation. This heuristic matches the long
comment at the bottom of `channel-t.h`, as one sentence includes the word `try`
after a linebreak.

Fix this misdetection by changing the comment to follow the convention that all
lines start with an asterisk.
---
 include/haproxy/channel-t.h | 184 ++--
 1 file changed, 92 insertions(+), 92 deletions(-)

diff --git a/include/haproxy/channel-t.h b/include/haproxy/channel-t.h
index 8bd06e147..8feb62dd2 100644
--- a/include/haproxy/channel-t.h
+++ b/include/haproxy/channel-t.h
@@ -207,98 +207,98 @@ struct channel {
 
 
 /* Note about the channel structure
-
-   A channel stores information needed to reliably transport data in a single
-   direction. It stores status flags, timeouts, counters, subscribed analysers,
-   pointers to a data producer and to a data consumer, and information about
-   the amount of data which is allowed to flow directly from the producer to
-   the consumer without waking up the analysers.
-
-   A channel may buffer data into two locations :
- - a visible buffer (->buf)
- - an invisible buffer which right now consists in a pipe making use of
-   kernel buffers that cannot be tampered with.
-
-   Data stored into the first location may be analysed and altered by analysers
-   while data stored in pipes is only aimed at being transported from one
-   network socket to another one without being subject to memory copies. This
-   buffer may only be used when both the socket layer and the data layer of the
-   producer and the consumer support it, which typically is the case with Linux
-   splicing over sockets, and when there are enough data to be transported
-   without being analyzed (transport of TCP/HTTP payload or tunnelled data,
-   which is indicated by ->to_forward).
-
-   In order not to mix data streams, the producer may only feed the invisible
-   data with data to forward, and only when the visible buffer is empty. The
-   producer may not always be able to feed the invisible buffer due to platform
-   limitations (lack of kernel support).
-
-   Conversely, the consumer must always take data from the invisible data first
-   before ever considering visible data. There is no limit to the size of data
-   to consume from the invisible buffer, as platform-specific implementations
-   will rarely leave enough control on this. So any byte fed into the invisible
-   buffer is expected to reach the destination file descriptor, by any means.
-   However, it's the consumer's responsibility to ensure that the invisible
-   data has been entirely consumed before consuming visible data. This must be
-   reflected by ->pipe->data. This is very important as this and only this can
-   ensure strict ordering of data between buffers.
-
-   The producer is responsible for decreasing ->to_forward. The ->to_forward
-   parameter indicates how many bytes may be fed into either data buffer
-   without waking the parent up. The special value CHN_INFINITE_FORWARD is
-   never decreased nor increased.
-
-   The buf->o parameter says how many bytes may be consumed from the visible
-   buffer. This parameter is updated by any buffer_write() as well as any data
-   forwarded through the visible buffer. Since the ->to_forward attribute
-   applies to data after buf->p, an analyser will not see a buffer which has a
-   non-null ->to_forward with buf->i > 0. A producer is responsible for raising
-   buf->o by min(to_forward, buf->i) when it injects data into the buffer.
-
-   The consumer is responsible for decreasing ->buf->o when it sends data
-   from the visible buffer, and ->pipe->data when it sends data from the
-   invisible buffer.
-
-   A real-world example consists in part in an HTTP response waiting in a
-   buffer to be forwarded. We know the header length (300) and the amount of
-   data to forward (content-length=9000). The buffer already contains 1000
-   bytes of data after the 300 bytes of headers. Thus the caller will set
-   buf->o to 300 indicating that it explicitly wants to send those data, and
-   set ->to_forward to 9000 (content-length). This value must be normalised
-   immediately after updating ->to_forward : since there are already 1300 bytes
-   in the buffer, 300 of which are already counted in buf->o, and that size
-   is smaller than ->to_forward, we must update buf->o to 1300 to flush the
-   whole buffer, and reduce ->to_forward to 8000. After that, the producer may
-   try to feed the additional data through the 

Re: [PATCH] CLEANUP: server: a separate function for initializing the per_thr field

[Willy Tarreau]

Hi Miroslav,

On Tue, Jun 15, 2021 at 03:53:39PM +0200, Miroslav Zagorac wrote:
> Hello all,
> 
> To avoid repeating the same source code, allocating memory and
> initializing the per_thr field from the server structure is
> transferred to a separate function.

Makes sense, indeed. Now merged, thanks!

> It seems to me that this code is only valid for the current version of
> haproxy, ie v2.4.

Well, current is now 2.5-dev and it's moving fast, and I had to apply
it by hand due to conflicting changes in the vicinity ;-)

Cheers,
Willy

[PATCH] CLEANUP: server: a separate function for initializing the per_thr field

[Miroslav Zagorac]

Hello all,

To avoid repeating the same source code, allocating memory and
initializing the per_thr field from the server structure is
transferred to a separate function.

It seems to me that this code is only valid for the current version of 
haproxy, ie v2.4.


Best regards,

--
Zaga

What can change the nature of a man?
>From 81ec1afe2a7da947952deedb394970248df6cd8b Mon Sep 17 00:00:00 2001
From: Miroslav Zagorac 
Date: Tue, 15 Jun 2021 15:33:20 +0200
Subject: [PATCH] CLEANUP: server: a separate function for initializing the
 per_thr field

To avoid repeating the same source code, allocating memory and initializing
the per_thr field from the server structure is transferred to a separate
function.
---
 include/haproxy/server.h |  1 +
 src/cfgparse.c   | 10 +-
 src/server.c | 32 ++--
 src/sink.c   | 11 +--
 4 files changed, 25 insertions(+), 29 deletions(-)

diff --git a/include/haproxy/server.h b/include/haproxy/server.h
index 74ac74053..b807a8a04 100644
--- a/include/haproxy/server.h
+++ b/include/haproxy/server.h
@@ -60,6 +60,7 @@ int srv_init_addr(void);
 struct server *cli_find_server(struct appctx *appctx, char *arg);
 struct server *new_server(struct proxy *proxy);
 void free_server(struct server *srv);
+int srv_init_per_thr(struct server *srv);
 
 /* functions related to server name resolution */
 int srv_prepare_for_resolution(struct server *srv, const char *hostname);
diff --git a/src/cfgparse.c b/src/cfgparse.c
index 906ab076f..800c61d24 100644
--- a/src/cfgparse.c
+++ b/src/cfgparse.c
@@ -3930,21 +3930,13 @@ int check_config_validity()
 
 	list_for_each_entry(newsrv, _list, global_list) {
 		/* initialize idle conns lists */
-		newsrv->per_thr = calloc(global.nbthread, sizeof(*newsrv->per_thr));
-		if (!newsrv->per_thr) {
+		if (srv_init_per_thr(newsrv) == -1) {
 			ha_alert("parsing [%s:%d] : failed to allocate per-thread lists for server '%s'.\n",
 			 newsrv->conf.file, newsrv->conf.line, newsrv->id);
 			cfgerr++;
 			continue;
 		}
 
-		for (i = 0; i < global.nbthread; i++) {
-			newsrv->per_thr[i].idle_conns = EB_ROOT;
-			newsrv->per_thr[i].safe_conns = EB_ROOT;
-			newsrv->per_thr[i].avail_conns = EB_ROOT;
-			MT_LIST_INIT(>per_thr[i].streams);
-		}
-
 		if (newsrv->max_idle_conns != 0) {
 			newsrv->curr_idle_thr = calloc(global.nbthread, sizeof(*newsrv->curr_idle_thr));
 			if (!newsrv->curr_idle_thr) {
diff --git a/src/server.c b/src/server.c
index 57b3feecb..2c55e320e 100644
--- a/src/server.c
+++ b/src/server.c
@@ -4308,6 +4308,27 @@ static int srv_alloc_lb(struct server *sv, struct proxy *be)
 	return 1;
 }
 
+/* Memory allocation and initialization of the per_thr field.
+ * Returns 0 if the field has been successfully initialized, -1 on failure.
+ */
+int srv_init_per_thr(struct server *srv)
+{
+	int i;
+
+	srv->per_thr = calloc(global.nbthread, sizeof(*srv->per_thr));
+	if (!srv->per_thr)
+		return -1;
+
+	for (i = 0; i < global.nbthread; i++) {
+		srv->per_thr[i].idle_conns = EB_ROOT;
+		srv->per_thr[i].safe_conns = EB_ROOT;
+		srv->per_thr[i].avail_conns = EB_ROOT;
+		MT_LIST_INIT(>per_thr[i].streams);
+	}
+
+	return 0;
+}
+
 /* Parse a "add server" command
  * Returns 0 if the server has been successfully initialized, 1 on failure.
  */
@@ -4317,7 +4338,6 @@ static int cli_parse_add_server(char **args, char *payload, struct appctx *appct
 	struct server *srv;
 	char *be_name, *sv_name;
 	int errcode, argc;
-	int i;
 	const int parse_flags = SRV_PARSE_DYNAMIC|SRV_PARSE_PARSE_ADDR;
 
 	usermsgs_clr("CLI");
@@ -4379,19 +4399,11 @@ static int cli_parse_add_server(char **args, char *payload, struct appctx *appct
 		}
 	}
 
-	srv->per_thr = calloc(global.nbthread, sizeof(*srv->per_thr));
-	if (!srv->per_thr) {
+	if (srv_init_per_thr(srv) == -1) {
 		ha_alert("failed to allocate per-thread lists for server.\n");
 		goto out;
 	}
 
-	for (i = 0; i < global.nbthread; i++) {
-		srv->per_thr[i].idle_conns = EB_ROOT;
-		srv->per_thr[i].safe_conns = EB_ROOT;
-		srv->per_thr[i].avail_conns = EB_ROOT;
-		MT_LIST_INIT(>per_thr[i].streams);
-	}
-
 	if (srv->max_idle_conns != 0) {
 		srv->curr_idle_thr = calloc(global.nbthread, sizeof(*srv->curr_idle_thr));
 		if (!srv->curr_idle_thr) {
diff --git a/src/sink.c b/src/sink.c
index 6358da53a..119c4e045 100644
--- a/src/sink.c
+++ b/src/sink.c
@@ -940,7 +940,6 @@ struct sink *sink_new_from_logsrv(struct logsrv *logsrv)
 	struct sink *sink = NULL;
 	struct server *srv = NULL;
 	struct sink_forward_target *sft = NULL;
-	int i;
 
 	/* allocate new proxy to handle
 	 * forward to a stream server
@@ -972,17 +971,9 @@ struct sink *sink_new_from_logsrv(struct logsrv *logsrv)
 	HA_SPIN_INIT(>lock);
 
 	/* process per thread init */
-	srv->per_thr = calloc(global.nbthread, sizeof(*srv->per_thr));
-	if (!srv->p

Re: [PATCH] CLEANUP: reg-tests: Remove obsolete no-htx parameter for reg-tests

[Willy Tarreau]

On Mon, May 31, 2021 at 11:07:29PM +0200, Tim Duesterhus wrote:
> The legacy HTTP subsystem has been removed. HTX is always enabled.

Now merged, thank you Tim!
Willy

[PATCH] CLEANUP: reg-tests: Remove obsolete no-htx parameter for reg-tests

[Tim Duesterhus]

The legacy HTTP subsystem has been removed. HTX is always enabled.
---
 reg-tests/README  |  2 +-
 reg-tests/cache/basic.vtc |  1 -
 reg-tests/cache/caching_rules.vtc |  1 -
 reg-tests/cache/expires.vtc   |  1 -
 reg-tests/cache/if-modified-since.vtc |  1 -
 reg-tests/cache/if-none-match.vtc |  1 -
 reg-tests/cache/post_on_entry.vtc |  1 -
 reg-tests/cache/sample_fetches.vtc|  1 -
 reg-tests/cache/vary.vtc  |  1 -
 reg-tests/cache/vary_accept_encoding.vtc  |  1 -
 reg-tests/compression/basic.vtc   |  1 -
 reg-tests/compression/etags_conversion.vtc|  1 -
 reg-tests/compression/lua_validation.vtc  |  1 -
 reg-tests/compression/vary.vtc|  1 -
 reg-tests/connection/dispatch.vtc |  1 -
 .../connection/proxy_protocol_random_fail.vtc |  1 -
 reg-tests/filters/random-forwarding.vtc   |  1 -
 reg-tests/http-capture/multiple_headers.vtc   |  1 -
 .../http-cookies/cookie_insert_indirect.vtc   |  1 -
 reg-tests/http-messaging/h1_to_h1.vtc |  1 -
 reg-tests/http-messaging/h2_to_h1.vtc |  1 -
 .../http-messaging/http_abortonclose.vtc  |  1 -
 .../http-messaging/http_bodyless_response.vtc |  1 -
 .../http-messaging/http_request_buffer.vtc|  1 -
 .../http-messaging/http_wait_for_body.vtc |  1 -
 reg-tests/http-messaging/protocol_upgrade.vtc |  1 -
 reg-tests/http-messaging/websocket.vtc|  3 ---
 reg-tests/http-rules/acl_cli_spaces.vtc   |  1 -
 ...erters_ipmask_concat_strcmp_field_word.vtc |  2 --
 reg-tests/http-rules/h1or2_to_h1c.vtc |  1 -
 reg-tests/http-rules/map_redirect.vtc |  1 -
 .../http-rules/map_regm_with_backref.vtc  |  1 -
 reg-tests/lua/bad_http_clt_req_duration.vtc   |  2 --
 reg-tests/lua/close_wait_lf.vtc   |  1 -
 reg-tests/lua/h_txn_get_priv.vtc  |  2 --
 reg-tests/lua/lua_socket.vtc  |  2 --
 reg-tests/lua/set_var.vtc |  2 --
 reg-tests/lua/txn_get_priv-thread.vtc |  3 ---
 reg-tests/lua/txn_get_priv.vtc|  3 ---
 reg-tests/lua/wrong_types_usage.vtc   |  2 --
 reg-tests/mcli/mcli_show_info.vtc |  1 -
 reg-tests/mcli/mcli_start_progs.vtc   |  1 -
 reg-tests/seamless-reload/abns_socket.vtc |  1 -
 reg-tests/server/cli_add_server.vtc   |  1 -
 reg-tests/server/cli_delete_server.vtc|  1 -
 reg-tests/server/cli_set_fdqn.vtc |  1 -
 reg-tests/server/cli_set_ssl.vtc  |  1 -
 reg-tests/ssl/add_ssl_crt-list.vtc|  2 --
 reg-tests/ssl/del_ssl_crt-list.vtc|  3 ---
 reg-tests/ssl/new_del_ssl_cafile.vtc  |  1 -
 reg-tests/ssl/new_del_ssl_crlfile.vtc |  1 -
 reg-tests/ssl/set_ssl_cafile.vtc  |  1 -
 reg-tests/ssl/set_ssl_cert.vtc|  1 -
 reg-tests/ssl/set_ssl_cert_bundle.vtc |  1 -
 reg-tests/ssl/set_ssl_cert_noext.vtc  |  1 -
 reg-tests/ssl/set_ssl_crlfile.vtc |  1 -
 reg-tests/ssl/set_ssl_server_cert.vtc |  1 -
 reg-tests/ssl/ssl_client_auth.vtc |  1 -
 reg-tests/ssl/ssl_client_samples.vtc  |  2 --
 reg-tests/ssl/ssl_crt-list_filters.vtc|  1 -
 reg-tests/ssl/ssl_frontend_samples.vtc|  2 --
 reg-tests/ssl/ssl_server_samples.vtc  |  2 --
 reg-tests/ssl/ssl_simple_crt-list.vtc |  1 -
 reg-tests/ssl/wrong_ctx_storage.vtc   |  1 -
 .../converteers_ref_cnt_never_dec.vtc |  1 -
 reg-tests/stick-table/src_conn_rate.vtc   |  1 -
 reg-tests/stick-table/unknown_key.vtc |  1 -
 reg-tests/stickiness/lb-services.vtc  |  2 --
 reg-tests/stickiness/srvkey-addr.vtc  |  2 --
 .../webstats-scope-and-post-change.vtc|  1 -
 scripts/run-regtests.sh   | 22 +--
 71 files changed, 2 insertions(+), 111 deletions(-)

diff --git a/reg-tests/README b/reg-tests/README
index d05a2a805..6993d9782 100644
--- a/reg-tests/README
+++ b/reg-tests/README
@@ -38,7 +38,7 @@ See also: doc/regression-testing.txt
   You must set HAPROXY_PROGRAM environment variable to give the location
   of the HAProxy program to test to vtest:
 
-$ HAPROXY_PROGRAM= vtest [-Dno-htx=] ...
+$ HAPROXY_PROGRAM= vtest ...
 
   The HAProxy VTC files found in HAProxy sources may be run with the reg-tests
   Makefile target. You must set the VTEST_PROGRAM environment variable to
diff --git a/reg-tests/cache/basic.vtc b/reg-tests/cache/basic.vtc
index 849057d9e..30f463108 100644
--- a/reg-tests/cache/basic.vtc
+++ b/reg-tests/cache/basic.vtc
@@ -27,7 +27,6 @@ server s1 {
 haproxy h1 -conf {
 defaults
 mode http
-${no-htx} option http-use-htx
 timeout connect 1s
 timeout client  1s
 timeout server  1s
diff --git a/reg-tests/cache/caching_rules.vtc 
b/reg-tests/cache/caching_rules.vtc

Re: [PATCH] CLEANUP: cli/activity: Remove double spacing in set profiling

[Willy Tarreau]

On Mon, May 10, 2021 at 03:52:26PM -0400, Daniel Corbett wrote:
> It was found that when viewing the help output from the CLI that
> "set profiling" had 2 spaces in it, which was pushing it out from
> the rest of similar commands.
> 
> i.e. it looked like this:
>   prepare acl 
>   prepare map 
>   set  profiling{auto|on|off}
>   set dynamic-cookie-key backend  
>   set map  [|#] 
>   set maxconn frontend  

Ah yes you're right. I noticed it before the sorting of the help and
left it as I found that in the end it used to align the words with
the previous "show". But now it's pointless and disturbing.

Seeing that we're discussing spaces in help messages is a good hint
that there's not that much dust left :-)

Applied, thanks!
Willy

[PATCH] CLEANUP: cli/activity: Remove double spacing in set profiling

[Daniel Corbett]

Hello,

 

 

It was found that when viewing the help output from the CLI that

"set profiling" had 2 spaces in it, which was pushing it out from

the rest of similar commands.

 

i.e. it looked like this:

  prepare acl 

  prepare map 

  set  profiling{auto|on|off}

  set dynamic-cookie-key backend  

  set map  [|#] 

  set maxconn frontend  

 

This patch removes all of the double spaces within the command and

unifies them to single spacing, which is what is observed within the

rest of the commands.

 

 

Thanks,

-- Daniel

 

 



0001-CLEANUP-cli-activity-Remove-double-spacing-in-set-pr.patch
Description: Binary data

Re: [PATCH] CLEANUP: Remove useless malloc() casts

[Willy Tarreau]

On Thu, Apr 08, 2021 at 08:05:23PM +0200, Tim Duesterhus wrote:
> Willy,
> 
> apparently I still had this one lying around. Found it while cleaning up local
> branches :-)

Ah yes that's indeed cleaner this way. Applied, thank you!
Willy

[PATCH] CLEANUP: Remove useless malloc() casts

[Tim Duesterhus]

Willy,

apparently I still had this one lying around. Found it while cleaning up local
branches :-)

Best regards
Tim Düsterhus

Apply with `git am --scissors` to automatically cut the commit message.

-- >8 --
This is not C++.
---
 include/haproxy/chunk.h| 2 +-
 include/haproxy/listener.h | 2 +-
 src/ev_select.c| 4 ++--
 src/http_htx.c | 2 +-
 4 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/include/haproxy/chunk.h b/include/haproxy/chunk.h
index b5052d135..af9ef816b 100644
--- a/include/haproxy/chunk.h
+++ b/include/haproxy/chunk.h
@@ -283,7 +283,7 @@ static inline char *chunk_dup(struct buffer *dst, const 
struct buffer *src)
if (dst->size < src->size || !src->size)
dst->size++;
 
-   dst->area = (char *)malloc(dst->size);
+   dst->area = malloc(dst->size);
if (!dst->area) {
dst->head = 0;
dst->data = 0;
diff --git a/include/haproxy/listener.h b/include/haproxy/listener.h
index e4cea50fc..17fe9e090 100644
--- a/include/haproxy/listener.h
+++ b/include/haproxy/listener.h
@@ -179,7 +179,7 @@ unsigned int bind_map_thread_id(const struct bind_conf 
*conf, unsigned int r);
 static inline struct bind_conf *bind_conf_alloc(struct proxy *fe, const char 
*file,
  int line, const char *arg, struct xprt_ops 
*xprt)
 {
-   struct bind_conf *bind_conf = (void *)calloc(1, sizeof(struct 
bind_conf));
+   struct bind_conf *bind_conf = calloc(1, sizeof(struct bind_conf));
 
bind_conf->file = strdup(file);
bind_conf->line = line;
diff --git a/src/ev_select.c b/src/ev_select.c
index 1e9c48cf5..ab5da35b7 100644
--- a/src/ev_select.c
+++ b/src/ev_select.c
@@ -226,10 +226,10 @@ static int init_select_per_thread()
int fd_set_bytes;
 
fd_set_bytes = sizeof(fd_set) * (global.maxsock + FD_SETSIZE - 1) / 
FD_SETSIZE;
-   tmp_evts[DIR_RD] = (fd_set *)calloc(1, fd_set_bytes);
+   tmp_evts[DIR_RD] = calloc(1, fd_set_bytes);
if (tmp_evts[DIR_RD] == NULL)
goto fail;
-   tmp_evts[DIR_WR] = (fd_set *)calloc(1, fd_set_bytes);
+   tmp_evts[DIR_WR] = calloc(1, fd_set_bytes);
if (tmp_evts[DIR_WR] == NULL)
goto fail;
return 1;
diff --git a/src/http_htx.c b/src/http_htx.c
index a7951c01d..53732d146 100644
--- a/src/http_htx.c
+++ b/src/http_htx.c
@@ -909,7 +909,7 @@ int http_str_to_htx(struct buffer *buf, struct ist raw, 
char **errmsg)
}
 
buf->size = global.tune.bufsize;
-   buf->area = (char *)malloc(buf->size);
+   buf->area = malloc(buf->size);
if (!buf->area)
goto error;
 
-- 
2.31.1

Re: Fwd: [PATCH] cleanup unused definitions

[William Lallemand]

On Wed, Mar 24, 2021 at 08:09:05AM +0100, Willy Tarreau wrote:
> William,
> 
> could you please have a quick look at these ? I didn't touch them because
> I don't know if these are things you intend to use in the short-to-mid
> term. If you take them, please be careful to fix the subject to mention
> "ssl" in them.
> 
> Thanks,
> Willy
> 
> - Forwarded message from  ???  -
> 
> > Date: Wed, 24 Mar 2021 11:29:03 +0500
> > From:  ??? 
> > Subject: Re: [PATCH] cleanup unused definitions
> > To: HAProxy , Willy Tarreau 
> > Delivered-To: haproxy@formilux.org
> > List-Id: Haproxy 
> > 
> > ping
> > 
> > ??, 20 ???. 2021 ?. ? 22:43,  ??? :
> > 
> > > while refactoring HA_OPENSSL_VERSION usage,
> > > I've found unused definitions. nice.
> > >
> > >
> > > Ilya
> > >
> 
> - End forwarded message -
> - Forwarded message from  ???  -
> 
> > Date: Wed, 24 Mar 2021 11:29:19 +0500
> > From:  ??? 
> > Subject: Re: [PATCH] BUILD: ssl: use feature guard instead of openssl 
> > version for ecdh functions
> > To: HAProxy , Willy Tarreau 
> > Delivered-To: haproxy@formilux.org
> > List-Id: Haproxy 
> > 
> > ping
> > 
> > ??, 21 ???. 2021 ?. ? 13:02,  ??? :
> > 
> > > Hello,
> > >
> > > yet another patch that reduces number of HA_OPENSSL_VERSION use
> > >
> > > Ilya
> > >
> > >
> > >
> 
> - End forwarded message -
> 

Thanks, both merged.

-- 
William Lallemand

Re: [PATCH] cleanup unused definitions

[William Lallemand]

On Wed, Mar 24, 2021 at 11:29:03AM +0500, Илья Шипицин wrote:
> ping
> 
> сб, 20 мар. 2021 г. в 22:43, Илья Шипицин :
> 
> > while refactoring HA_OPENSSL_VERSION usage,
> > I've found unused definitions. nice.
> >
> >
> > Ilya
> >

Thanks, merged.


-- 
William Lallemand

Fwd: [PATCH] cleanup unused definitions

[Willy Tarreau]

William,

could you please have a quick look at these ? I didn't touch them because
I don't know if these are things you intend to use in the short-to-mid
term. If you take them, please be careful to fix the subject to mention
"ssl" in them.

Thanks,
Willy

- Forwarded message from  ???  -

> Date: Wed, 24 Mar 2021 11:29:03 +0500
> From:  ??? 
> Subject: Re: [PATCH] cleanup unused definitions
> To: HAProxy , Willy Tarreau 
> Delivered-To: haproxy@formilux.org
> List-Id: Haproxy 
> 
> ping
> 
> ??, 20 ???. 2021 ?. ? 22:43,  ??? :
> 
> > while refactoring HA_OPENSSL_VERSION usage,
> > I've found unused definitions. nice.
> >
> >
> > Ilya
> >

- End forwarded message -
- Forwarded message from  ???  -

> Date: Wed, 24 Mar 2021 11:29:19 +0500
> From:  ??? 
> Subject: Re: [PATCH] BUILD: ssl: use feature guard instead of openssl version 
> for ecdh functions
> To: HAProxy , Willy Tarreau 
> Delivered-To: haproxy@formilux.org
> List-Id: Haproxy 
> 
> ping
> 
> ??, 21 ???. 2021 ?. ? 13:02,  ??? :
> 
> > Hello,
> >
> > yet another patch that reduces number of HA_OPENSSL_VERSION use
> >
> > Ilya
> >
> >
> >

- End forwarded message -

Re: [PATCH] cleanup unused definitions

[Илья Шипицин]

ping

сб, 20 мар. 2021 г. в 22:43, Илья Шипицин :

> while refactoring HA_OPENSSL_VERSION usage,
> I've found unused definitions. nice.
>
>
> Ilya
>

[PATCH] cleanup unused definitions

[Илья Шипицин]

while refactoring HA_OPENSSL_VERSION usage,
I've found unused definitions. nice.


Ilya
From c5ba9de13cb7d5eda4781731bb4bfb68c823114c Mon Sep 17 00:00:00 2001
From: Ilya Shipitsin 
Date: Sat, 20 Mar 2021 22:30:59 +0500
Subject: [PATCH] CLEANUP: remove unused definitions

not need since 	e7eb1fec2f2349359c752c8fbb82357b14c7e4cf
---
 include/haproxy/ssl_sock-t.h | 20 
 1 file changed, 20 deletions(-)

diff --git a/include/haproxy/ssl_sock-t.h b/include/haproxy/ssl_sock-t.h
index c0f47cbb6..991a08b3d 100644
--- a/include/haproxy/ssl_sock-t.h
+++ b/include/haproxy/ssl_sock-t.h
@@ -175,26 +175,6 @@ struct sh_ssl_sess_hdr {
 	unsigned char key_data[SSL_MAX_SSL_SESSION_ID_LENGTH];
 };
 
-#if HA_OPENSSL_VERSION_NUMBER >= 0x1000200fL
-
-#define SSL_SOCK_POSSIBLE_KT_COMBOS (1<<(SSL_SOCK_NUM_KEYTYPES))
-
-struct key_combo_ctx {
-	SSL_CTX *ctx;
-	int order;
-};
-
-/* Map used for processing multiple keypairs for a single purpose
- *
- * This maps CN/SNI name to certificate type
- */
-struct sni_keytype {
-	int keytypes;			  /* BITMASK for keytypes */
-	struct ebmb_node name;/* node holding the servername value */
-};
-
-#endif
-
 /* issuer chain store with hash of Subject Key Identifier
certificate/issuer matching is verify with X509_check_issued
 */
-- 
2.29.2

Re: [PATCH] cleanup #1162 coverity "finding"

[Илья Шипицин]

OK, I will mark as false positive

On Sat, Mar 20, 2021, 1:42 PM Willy Tarreau  wrote:

> Hi Ilya,
>
> On Fri, Mar 19, 2021 at 11:19:14PM +0500,  ??? wrote:
> > Hello,
> >
> > pure cleanup, should fix #1162
> >
> > Ilya
>
> > From 7915f3ed189e0db371699fa8c8eb74a69584ed27 Mon Sep 17 00:00:00 2001
> > From: Ilya Shipitsin 
> > Date: Fri, 19 Mar 2021 23:06:36 +0500
> > Subject: [PATCH] CLEANUP: move "tmp_ssl" definition and usage only when
> it is
> >  required
> >
> > if we do not have HAVE_SSL_CTX_get0_privatekey defined, we need to
> extract
> > private key manually, via "tmp_ssl" intermediate variable.
> >
> > it is not needed otherwise.
> > this should fix #1162
>
> Thanks but this in fact adds burden to the code's maintenance and will
> inevitably result in new build warnings as soon as someone adds stuff
> above the first ifdef. Plus it's extremely prone to use-after-free bugs
> by being freed in the middle of the function without the pointer being
> reset.
>
> The real issue here is that Coverity cannot imagine all valid ifdef
> combinations and only evaluates one path and considers there's dead
> code. But that's precisely the purpose of such clean constructs, to
> keep the code auditable and maintainable while relying on the compiler
> to reliminate possible dead code.
>
> Let's rather mark it as a false positive. I'm even wondering if we've
> ever got a real bug reported via deadcode, because as soon as you
> start to support ifdefs and various build options, you'll get lots of
> situations resulting in apparently dead code; as such I'm suspecting
> that we'd rather completely disable the deadcode checker which probably
> only reports 100% noise.
>
> Thanks!
> Willy
>

Re: [PATCH] cleanup #1162 coverity "finding"

[Willy Tarreau]

Hi Ilya,

On Fri, Mar 19, 2021 at 11:19:14PM +0500,  ??? wrote:
> Hello,
> 
> pure cleanup, should fix #1162
> 
> Ilya

> From 7915f3ed189e0db371699fa8c8eb74a69584ed27 Mon Sep 17 00:00:00 2001
> From: Ilya Shipitsin 
> Date: Fri, 19 Mar 2021 23:06:36 +0500
> Subject: [PATCH] CLEANUP: move "tmp_ssl" definition and usage only when it is
>  required
> 
> if we do not have HAVE_SSL_CTX_get0_privatekey defined, we need to extract
> private key manually, via "tmp_ssl" intermediate variable.
> 
> it is not needed otherwise.
> this should fix #1162

Thanks but this in fact adds burden to the code's maintenance and will
inevitably result in new build warnings as soon as someone adds stuff
above the first ifdef. Plus it's extremely prone to use-after-free bugs
by being freed in the middle of the function without the pointer being
reset.

The real issue here is that Coverity cannot imagine all valid ifdef
combinations and only evaluates one path and considers there's dead
code. But that's precisely the purpose of such clean constructs, to
keep the code auditable and maintainable while relying on the compiler
to reliminate possible dead code.

Let's rather mark it as a false positive. I'm even wondering if we've
ever got a real bug reported via deadcode, because as soon as you
start to support ifdefs and various build options, you'll get lots of
situations resulting in apparently dead code; as such I'm suspecting
that we'd rather completely disable the deadcode checker which probably
only reports 100% noise.

Thanks!
Willy

[PATCH] cleanup #1162 coverity "finding"

[Илья Шипицин]

Hello,

pure cleanup, should fix #1162

Ilya
From 7915f3ed189e0db371699fa8c8eb74a69584ed27 Mon Sep 17 00:00:00 2001
From: Ilya Shipitsin 
Date: Fri, 19 Mar 2021 23:06:36 +0500
Subject: [PATCH] CLEANUP: move "tmp_ssl" definition and usage only when it is
 required

if we do not have HAVE_SSL_CTX_get0_privatekey defined, we need to extract
private key manually, via "tmp_ssl" intermediate variable.

it is not needed otherwise.

this should fix #1162
---
 src/ssl_sock.c | 7 ---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/src/ssl_sock.c b/src/ssl_sock.c
index debd05e6f..eff1f9761 100644
--- a/src/ssl_sock.c
+++ b/src/ssl_sock.c
@@ -1913,7 +1913,6 @@ ssl_sock_do_create_cert(const char *servername, struct bind_conf *bind_conf, SSL
 	SSL_CTX  *ssl_ctx = NULL;
 	X509 *newcrt  = NULL;
 	EVP_PKEY *pkey= NULL;
-	SSL  *tmp_ssl = NULL;
 	CONF *ctmp= NULL;
 	X509_NAME*name;
 	const EVP_MD *digest;
@@ -1925,9 +1924,12 @@ ssl_sock_do_create_cert(const char *servername, struct bind_conf *bind_conf, SSL
 #ifdef HAVE_SSL_CTX_get0_privatekey
 	pkey = SSL_CTX_get0_privatekey(bind_conf->default_ctx);
 #else
+	SSL  *tmp_ssl = NULL;
 	tmp_ssl = SSL_new(bind_conf->default_ctx);
-	if (tmp_ssl)
+	if (tmp_ssl) {
 		pkey = SSL_get_privatekey(tmp_ssl);
+		SSL_free(tmp_ssl);
+	}
 #endif
 	if (!pkey)
 		goto mkcert_error;
@@ -2067,7 +2069,6 @@ ssl_sock_do_create_cert(const char *servername, struct bind_conf *bind_conf, SSL
 
  mkcert_error:
 	if (ctmp) NCONF_free(ctmp);
-	if (tmp_ssl) SSL_free(tmp_ssl);
 	if (ssl_ctx) SSL_CTX_free(ssl_ctx);
 	if (newcrt)  X509_free(newcrt);
 	return NULL;
-- 
2.29.2

Re: [PATCH] CLEANUP: Use the ist() macro whenever possible

[Willy Tarreau]

Both cleanups applied, thank you Tim!
Willy

[PATCH] CLEANUP: Replace for loop with only a condition by while

[Tim Duesterhus]

Refactoring performed with the following Coccinelle patch:

@@
expression e;
statement S;
@@

- for (;e;)
+ while (e)
  S
---
 src/h2.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/h2.c b/src/h2.c
index 84aa660b7..9ff3c938e 100644
--- a/src/h2.c
+++ b/src/h2.c
@@ -535,7 +535,7 @@ int h2_make_htx_request(struct http_hdr *list, struct htx 
*htx, unsigned int *ms
 * insert "; " before the new value.
 */
fs += tl; // first one is already counted
-   for (; (ck = list[ck].n.len) >= 0 ; ) {
+   while ((ck = list[ck].n.len) >= 0) {
vl = list[ck].v.len;
tl += vl + 2;
if (tl > fs)
-- 
2.30.1

[PATCH] CLEANUP: Use the ist() macro whenever possible

[Tim Duesterhus]

Refactoring performed with the following Coccinelle patch:

@@
char *s;
@@

(
- ist2(s, strlen(s))
+ ist(s)
|
- ist2(strdup(s), strlen(s))
+ ist(strdup(s))
)

Note that this replacement is safe even in the strdup() case, because `ist()`
will not call `strlen()` on a `NULL` pointer. Instead is inserts a length of
`0`, effectively resulting in `IST_NULL`.
---
 src/check.c|  2 +-
 src/fcgi-app.c |  4 ++--
 src/http_ana.c |  2 +-
 src/listener.c |  2 +-
 src/mux_fcgi.c |  4 ++--
 src/mux_h1.c   |  2 +-
 src/server.c   |  2 +-
 src/stats.c|  2 +-
 src/tcpcheck.c | 28 ++--
 9 files changed, 24 insertions(+), 24 deletions(-)

diff --git a/src/check.c b/src/check.c
index 0fab0d412..ba9c1f315 100644
--- a/src/check.c
+++ b/src/check.c
@@ -1851,7 +1851,7 @@ static int srv_parse_check_proto(char **args, int 
*cur_arg,
memprintf(err, "'%s' : missing value", args[*cur_arg]);
goto error;
}
-   newsrv->check.mux_proto = get_mux_proto(ist2(args[*cur_arg + 1], 
strlen(args[*cur_arg + 1])));
+   newsrv->check.mux_proto = get_mux_proto(ist(args[*cur_arg + 1]));
if (!newsrv->check.mux_proto) {
memprintf(err, "'%s' :  unknown MUX protocol '%s'", 
args[*cur_arg], args[*cur_arg+1]);
goto error;
diff --git a/src/fcgi-app.c b/src/fcgi-app.c
index f4ce40c21..71f6dd48b 100644
--- a/src/fcgi-app.c
+++ b/src/fcgi-app.c
@@ -854,7 +854,7 @@ static int cfg_parse_fcgi_app(const char *file, int 
linenum, char **args, int kw
if (alertif_too_many_args_idx(0, 1, file, linenum, args, 
_code))
goto out;
istfree(>docroot);
-   curapp->docroot = ist2(strdup(args[1]), strlen(args[1]));
+   curapp->docroot = ist(strdup(args[1]));
if (!isttest(curapp->docroot)) {
ha_alert("parsing [%s:%d] : out of memory.\n", file, 
linenum);
err_code |= ERR_ALERT | ERR_ABORT;
@@ -887,7 +887,7 @@ static int cfg_parse_fcgi_app(const char *file, int 
linenum, char **args, int kw
if (alertif_too_many_args_idx(0, 1, file, linenum, args, 
_code))
goto out;
istfree(>index);
-   curapp->index = ist2(strdup(args[1]), strlen(args[1]));
+   curapp->index = ist(strdup(args[1]));
if (!isttest(curapp->index)) {
ha_alert("parsing [%s:%d] : out of memory.\n", file, 
linenum);
err_code |= ERR_ALERT | ERR_ABORT;
diff --git a/src/http_ana.c b/src/http_ana.c
index 5536c0402..c1245442f 100644
--- a/src/http_ana.c
+++ b/src/http_ana.c
@@ -2733,7 +2733,7 @@ int http_res_set_status(unsigned int status, struct ist 
reason, struct stream *s
/* Do we have a custom reason format string? */
if (!isttest(reason)) {
const char *str = http_get_reason(status);
-   reason = ist2(str, strlen(str));
+   reason = ist(str);
}
 
if (!http_replace_res_status(htx, ist2(trash.area, trash.data), reason))
diff --git a/src/listener.c b/src/listener.c
index 48ba8e0e4..400a7ebc2 100644
--- a/src/listener.c
+++ b/src/listener.c
@@ -1496,7 +1496,7 @@ static int bind_parse_proto(char **args, int cur_arg, 
struct proxy *px, struct b
return ERR_ALERT | ERR_FATAL;
}
 
-   proto = ist2(args[cur_arg + 1], strlen(args[cur_arg + 1]));
+   proto = ist(args[cur_arg + 1]);
conf->mux_proto = get_mux_proto(proto);
if (!conf->mux_proto) {
memprintf(err, "'%s' :  unknown MUX protocol '%s'", 
args[cur_arg], args[cur_arg+1]);
diff --git a/src/mux_fcgi.c b/src/mux_fcgi.c
index 70b3d2272..fabc83e90 100644
--- a/src/mux_fcgi.c
+++ b/src/mux_fcgi.c
@@ -1269,7 +1269,7 @@ static int fcgi_set_default_param(struct fcgi_conn 
*fconn, struct fcgi_strm *fst
if (addr_to_str(cli_conn->dst, 
b_tail(params->p), b_room(params->p)) != -1)
ptr = b_tail(params->p);
if (ptr) {
-   params->srv_name = ist2(ptr, strlen(ptr));
+   params->srv_name = ist(ptr);
params->p->data += params->srv_name.len;
}
}
@@ -1281,7 +1281,7 @@ static int fcgi_set_default_param(struct fcgi_conn 
*fconn, struct fcgi_strm *fst
if (addr_to_str(cli_conn->src, b_tail(params->p), 
b_room(params->p)) != -1)
ptr = b_tail(params->p);
if (ptr) {
-   params->rem_addr = ist2(ptr, strlen(ptr));
+   params->rem_addr = ist(ptr);
params->p->data += params->rem_addr.len;
}
}
diff --git a/src/mux_h1.c b/src/mux_h1.c
index 

Re: [PATCH] cleanup: remove unused variable assignment

[Christopher Faulet]

Le 10/02/2021 à 09:09, Илья Шипицин a écrit :

Hello,

this is pure cleanup.
should fix #1048



Thanks Ilya, now merged !

--
Christopher Faulet

[PATCH] cleanup: remove unused variable assignment

[Илья Шипицин]

Hello,

this is pure cleanup.
should fix #1048

Ilya
From 350c37117369607928ddb232cf8f86b1c218bd47 Mon Sep 17 00:00:00 2001
From: Ilya Shipitsin 
Date: Wed, 10 Feb 2021 13:03:30 +0500
Subject: [PATCH] CLEANUP: remove unused variable assigned found by Coverity

this is pure cleanup, no need to backport

2116if ((end - 1) == (payload + strlen(PAYLOAD_PATTERN))) {
2117/* if the payload pattern is at the end */
2118s->pcli_flags |= PCLI_F_PAYLOAD;
CID 1399833 (#1 of 1): Unused value (UNUSED_VALUE)assigned_value: Assigning value from reql to ret here, but that stored value is overwritten before it can be used.
2119ret = reql;
2120}
---
 src/cli.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/src/cli.c b/src/cli.c
index bfce9a60b..8b32e7610 100644
--- a/src/cli.c
+++ b/src/cli.c
@@ -2184,7 +2184,6 @@ int pcli_parse_request(struct stream *s, struct channel *req, char **errmsg, int
 	if ((end - 1) == (payload + strlen(PAYLOAD_PATTERN))) {
 		/* if the payload pattern is at the end */
 		s->pcli_flags |= PCLI_F_PAYLOAD;
-		ret = reql;
 	}
 
 	*(end-1) = '\n';
-- 
2.29.2

Re: [PATCH] CLEANUP: sample: remove uneeded check in json validation

[Willy Tarreau]

On Fri, Jan 08, 2021 at 09:57:41PM +0100, William Dauchy wrote:
> - check functions are never called with a NULL args list, it is always
>   an array, so first check can be removed
> - the expression parser guarantees that we can't have anything else,
>   because we mentioned json converter takes a mandatory string argument.
>   Thus test on `ARGT_STR` can be removed as well
> - also add breaking line between enum and function declaration
(...)

Merged, thanks you William!
Willy

[PATCH] CLEANUP: sample: remove uneeded check in json validation

[William Dauchy]

- check functions are never called with a NULL args list, it is always
  an array, so first check can be removed
- the expression parser guarantees that we can't have anything else,
  because we mentioned json converter takes a mandatory string argument.
  Thus test on `ARGT_STR` can be removed as well
- also add breaking line between enum and function declaration

In order to validate it, add a simple json test testing very simple
cases but can be improved in the future:

- default json converter without args
- json converter failing on error (utf8)
- json converter with error being removed (utf8s)

Signed-off-by: William Dauchy 
---
 reg-tests/converter/json.vtc | 41 
 src/sample.c | 11 +-
 2 files changed, 42 insertions(+), 10 deletions(-)
 create mode 100644 reg-tests/converter/json.vtc

diff --git a/reg-tests/converter/json.vtc b/reg-tests/converter/json.vtc
new file mode 100644
index 0..efac8f622
--- /dev/null
+++ b/reg-tests/converter/json.vtc
@@ -0,0 +1,41 @@
+varnishtest "json converter test"
+
+#REQUIRE_VERSION=1.6
+
+feature ignore_unknown_macro
+
+server s1 {
+   rxreq
+   txresp
+} -repeat 2 -start
+
+haproxy h1 -conf {
+defaults
+   mode http
+   timeout connect 1s
+   timeout client  1s
+   timeout server  1s
+
+frontend fe
+   bind "fd@${fe}"
+
+   http-response set-header json0 "%[str(foo 1/2),json]"
+   # bad UTF-8 sequence
+   http-response set-header json1 "%[str(\xE0),json(utf8)]"
+   # bad UTF-8 sequence, but removes them
+   http-response set-header json2 "%[str(-\xE0-),json(utf8s)]"
+
+   default_backend be
+
+backend be
+   server s1 ${s1_addr}:${s1_port}
+} -start
+
+client c1 -connect ${h1_fe_sock} {
+   txreq -url "/"
+   rxresp
+   expect resp.http.json0 == "foo 1\\/2"
+   expect resp.http.json1 == ""
+   expect resp.http.json2 == "--"
+   expect resp.status == 200
+} -run
diff --git a/src/sample.c b/src/sample.c
index 41d95d86c..fa854632f 100644
--- a/src/sample.c
+++ b/src/sample.c
@@ -2152,21 +2152,12 @@ enum input_type {
IT_UTF8P,
IT_UTF8PS,
 };
+
 static int sample_conv_json_check(struct arg *arg, struct sample_conv *conv,
   const char *file, int line, char **err)
 {
enum input_type type;
 
-   if (!arg) {
-   memprintf(err, "Unexpected empty arg list");
-   return 0;
-   }
-
-   if (arg->type != ARGT_STR) {
-   memprintf(err, "Unexpected arg type");
-   return 0;
-   }
-
if (strcmp(arg->data.str.area, "") == 0)
type = IT_ASCII;
else if (strcmp(arg->data.str.area, "ascii") == 0)
-- 
2.29.2

Re: [PATCH] cleanup: switch to my_realloc2 instead of realloc

[Willy Tarreau]

On Thu, Jan 07, 2021 at 10:59:26PM +0500,  ??? wrote:
> patch resolving #1030 attached.

Applied, thanks Ilya!
Willy

[PATCH] cleanup: switch to my_realloc2 instead of realloc

[Илья Шипицин]

Hi,

patch resolving #1030 attached.

Ilya
From eb849f02b0fef02a4d40f7566da3b54b2d0a8368 Mon Sep 17 00:00:00 2001
From: Ilya Shipitsin 
Date: Thu, 7 Jan 2021 22:45:13 +0500
Subject: [PATCH] CLEANUP: replace "realloc" with "my_realloc2" to fix to
 memory leak

my_realloc2 frees variable in case of allocation failure

fixes #1030

realloc was introduced in 9e1758efbd68c8b1d27e17e2abee110f3ebe

this might be backported to 2.2, 2.3
---
 src/cfgparse.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/cfgparse.c b/src/cfgparse.c
index c34b3a8f5..12eae67cc 100644
--- a/src/cfgparse.c
+++ b/src/cfgparse.c
@@ -1983,7 +1983,7 @@ next_line:
 
 			if (err & (PARSE_ERR_TOOLARGE|PARSE_ERR_OVERLAP)) {
 outlinesize = (outlen + 1023) & -1024;
-outline = realloc(outline, outlinesize);
+outline = my_realloc2(outline, outlinesize);
 if (outline == NULL) {
 	ha_alert("parsing [%s:%d]: line too long, cannot allocate memory.\n",
 		 file, linenum);
-- 
2.29.2

Re: [PATCH] CLEANUP: spoe: fix typo on `var_check_arg` comment

[Willy Tarreau]

On Tue, Jan 05, 2021 at 11:14:58AM +0100, William Dauchy wrote:
> there was an extra `s` added to the `var_check_arg` function

Merged, thank you William.
Willy

[PATCH] CLEANUP: spoe: fix typo on `var_check_arg` comment

[William Dauchy]

there was an extra `s` added to the `var_check_arg` function

Signed-off-by: William Dauchy 
---
 src/flt_spoe.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/src/flt_spoe.c b/src/flt_spoe.c
index 45370fee6..bfd6f4fbf 100644
--- a/src/flt_spoe.c
+++ b/src/flt_spoe.c
@@ -4178,7 +4178,7 @@ parse_spoe_flt(char **args, int *cur_arg, struct proxy 
*px,
arg.type = ARGT_STR;
arg.data.str.area = trash.area;
arg.data.str.data = trash.data;
-   arg.data.str.size = 0; /* Set it to 0 to not release it in 
vars_check_args() */
+   arg.data.str.size = 0; /* Set it to 0 to not release it in 
vars_check_arg() */
if (!vars_check_arg(, err)) {
memprintf(err, "SPOE agent '%s': failed to register 
variable %s.%s (%s)",
  curagent->id, curagent->var_pfx, 
curagent->var_on_error, *err);
@@ -4195,7 +4195,7 @@ parse_spoe_flt(char **args, int *cur_arg, struct proxy 
*px,
arg.type = ARGT_STR;
arg.data.str.area = trash.area;
arg.data.str.data = trash.data;
-   arg.data.str.size = 0;  /* Set it to 0 to not release it in 
vars_check_args() */
+   arg.data.str.size = 0;  /* Set it to 0 to not release it in 
vars_check_arg() */
if (!vars_check_arg(, err)) {
memprintf(err, "SPOE agent '%s': failed to register 
variable %s.%s (%s)",
  curagent->id, curagent->var_pfx, 
curagent->var_t_process, *err);
@@ -4212,7 +4212,7 @@ parse_spoe_flt(char **args, int *cur_arg, struct proxy 
*px,
arg.type = ARGT_STR;
arg.data.str.area = trash.area;
arg.data.str.data = trash.data;
-   arg.data.str.size = 0;  /* Set it to 0 to not release it in 
vars_check_args() */
+   arg.data.str.size = 0;  /* Set it to 0 to not release it in 
vars_check_arg() */
if (!vars_check_arg(, err)) {
memprintf(err, "SPOE agent '%s': failed to register 
variable %s.%s (%s)",
  curagent->id, curagent->var_pfx, 
curagent->var_t_process, *err);
@@ -4418,7 +4418,7 @@ parse_spoe_flt(char **args, int *cur_arg, struct proxy 
*px,
arg.type = ARGT_STR;
arg.data.str.area = trash.area;
arg.data.str.data = trash.data;
-   arg.data.str.size = 0;  /* Set it to 0 to not release it in 
vars_check_args() */
+   arg.data.str.size = 0;  /* Set it to 0 to not release it in 
vars_check_arg() */
if (!vars_check_arg(, err)) {
memprintf(err, "SPOE agent '%s': failed to register 
variable %s.%s (%s)",
  curagent->id, curagent->var_pfx, vph->name, 
*err);
-- 
2.29.2

Re: [PATCH] CLEANUP: contrib/prometheus-exporter: typo fixes for ssl reuse metric

[Willy Tarreau]

Hi William,

On Wed, Dec 16, 2020 at 12:52:57PM +0100, William Dauchy wrote:
> Hi Willy
> 
> On Tue, Jul 7, 2020 at 11:17 PM Willy Tarreau  wrote:
> > On Tue, Jul 07, 2020 at 07:14:08PM +0200, Pierre Cheynier wrote:
> > > A typo I identified while having a look to our metric inventory.
> >
> > Thank you, now merged.
> 
> Do you think that's something we could backport to v2.x to make it
> coherent across versions?

Sure, it makes sense! I guess it was skipped because it was tagged
as a cleanup but in fact it fixes a real bug since the output metric
name was fixed. I backported it to 2.2 right now so it's in the pipe
now :-)

Thanks,
Willy

Re: [PATCH] CLEANUP: contrib/prometheus-exporter: typo fixes for ssl reuse metric

[William Dauchy]

Hi Willy

On Tue, Jul 7, 2020 at 11:17 PM Willy Tarreau  wrote:
> On Tue, Jul 07, 2020 at 07:14:08PM +0200, Pierre Cheynier wrote:
> > A typo I identified while having a look to our metric inventory.
>
> Thank you, now merged.

Do you think that's something we could backport to v2.x to make it
coherent across versions?

Thanks,
-- 
William

Re: [PATCH] CLEANUP: http_ana: remove unused assignation of `att_beg`

[Christopher Faulet]

Le 25/10/2020 à 14:01, William Dauchy a écrit :

`att_beg` is assigned to `next` at the end of the `for` loop, but is
assigned to `prev` at the beginning of the loop, which is itself
assigned to `next` after each loop. So it represents a double
assignation for the same value. Also `att_beg` is not used after the end
of the loop.

this is a partial fix for github issue #923, all the others could
probably be marked as intentional to protect future changes.

no backport needed.

Signed-off-by: William Dauchy 
---
  src/http_ana.c | 2 --
  1 file changed, 2 deletions(-)

diff --git a/src/http_ana.c b/src/http_ana.c
index f2edc681f..c2080bf9a 100644
--- a/src/http_ana.c
+++ b/src/http_ana.c
@@ -3533,8 +3533,6 @@ static void http_manage_client_side_cookies(struct stream 
*s, struct channel *re
}
}
  
-			/* continue with next cookie on this header line */

-   att_beg = next;
} /* for each cookie */
  
  



Thanks William, now applied.

--
Christopher Faulet

[PATCH] CLEANUP: http_ana: remove unused assignation of `att_beg`

[William Dauchy]

`att_beg` is assigned to `next` at the end of the `for` loop, but is
assigned to `prev` at the beginning of the loop, which is itself
assigned to `next` after each loop. So it represents a double
assignation for the same value. Also `att_beg` is not used after the end
of the loop.

this is a partial fix for github issue #923, all the others could
probably be marked as intentional to protect future changes.

no backport needed.

Signed-off-by: William Dauchy 
---
 src/http_ana.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/src/http_ana.c b/src/http_ana.c
index f2edc681f..c2080bf9a 100644
--- a/src/http_ana.c
+++ b/src/http_ana.c
@@ -3533,8 +3533,6 @@ static void http_manage_client_side_cookies(struct stream 
*s, struct channel *re
}
}
 
-   /* continue with next cookie on this header line */
-   att_beg = next;
} /* for each cookie */
 
 
-- 
2.28.0

Re: [PATCH] CLEANUP: Update .gitignore

[Willy Tarreau]

On Sun, Sep 06, 2020 at 06:34:12PM +0200, Tim Duesterhus wrote:
> This excludes the currently tracked files from being ignored.

Good spot, now merged, thanks!
Willy

[PATCH] CLEANUP: Update .gitignore

[Tim Duesterhus]

This excludes the currently tracked files from being ignored.
---
 .gitignore | 4 
 1 file changed, 4 insertions(+)

diff --git a/.gitignore b/.gitignore
index 7e45489d2..3a760af99 100644
--- a/.gitignore
+++ b/.gitignore
@@ -2,7 +2,11 @@
 # than blocking about 500 different test files and bug report outputs.
 /.*
 /*
+!/.cirrus.yml
+!/.gitattributes
+!/.github
 !/.gitignore
+!/.travis.yml
 !/CHANGELOG
 !/LICENSE
 !/BRANCHES
-- 
2.28.0

Re: [PATCH] CLEANUP: fix all duplicated semicolons

[William Lallemand]

On Fri, Aug 07, 2020 at 10:19:23PM +0200, William Dauchy wrote:
> trivial commit, does not change the code behaviour
> 

Thanks, merged!

-- 
William Lallemand

[PATCH] CLEANUP: fix all duplicated semicolons

[William Dauchy]

trivial commit, does not change the code behaviour

Signed-off-by: William Dauchy 
---
 src/mux_fcgi.c | 2 +-
 src/mux_h2.c   | 2 +-
 src/ring.c | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/src/mux_fcgi.c b/src/mux_fcgi.c
index 875049884..ab9ddf095 100644
--- a/src/mux_fcgi.c
+++ b/src/mux_fcgi.c
@@ -825,7 +825,7 @@ static inline struct fcgi_strm *fcgi_conn_st_by_id(struct 
fcgi_conn *fconn, int
  */
 static void fcgi_release(struct fcgi_conn *fconn)
 {
-   struct connection *conn = NULL;;
+   struct connection *conn = NULL;
 
TRACE_POINT(FCGI_EV_FCONN_END);
 
diff --git a/src/mux_h2.c b/src/mux_h2.c
index 44955ba8f..bfa65bd24 100644
--- a/src/mux_h2.c
+++ b/src/mux_h2.c
@@ -926,7 +926,7 @@ static inline struct h2s *h2c_st_by_id(struct h2c *h2c, int 
id)
  */
 static void h2_release(struct h2c *h2c)
 {
-   struct connection *conn = NULL;;
+   struct connection *conn = NULL;
 
TRACE_ENTER(H2_EV_H2C_END);
 
diff --git a/src/ring.c b/src/ring.c
index 7c7d58edc..2ee6c10b9 100644
--- a/src/ring.c
+++ b/src/ring.c
@@ -186,7 +186,7 @@ ssize_t ring_write(struct ring *ring, size_t maxlen, const 
struct ist pfx[], siz
totlen += len;
}
 
-   *b_tail(buf) = 0; buf->data++;; // new read counter
+   *b_tail(buf) = 0; buf->data++; // new read counter
sent = lenlen + totlen + 1;
 
/* notify potential readers */
-- 
2.28.0

Re: [PATCH] CLEANUP: contrib/prometheus-exporter: typo fixes for ssl reuse metric

[Willy Tarreau]

Hi Pierre,

On Tue, Jul 07, 2020 at 07:14:08PM +0200, Pierre Cheynier wrote:
> A typo I identified while having a look to our metric inventory.

Thank you, now merged.

Willy

[PATCH] CLEANUP: contrib/prometheus-exporter: typo fixes for ssl reuse metric

[Pierre Cheynier]

A typo I identified while having a look to our metric inventory.

---
 contrib/prometheus-exporter/README   | 2 +-
 contrib/prometheus-exporter/service-prometheus.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/contrib/prometheus-exporter/README 
b/contrib/prometheus-exporter/README
index 1c5a99241..a1b9e269c 100644
--- a/contrib/prometheus-exporter/README
+++ b/contrib/prometheus-exporter/README
@@ -122,7 +122,7 @@ Exported metrics
 | haproxy_process_max_ssl_rate   | Maximum observed number of 
SSL sessions per second.   |
 | haproxy_process_current_frontend_ssl_key_rate  | Current frontend SSL Key 
computation per second over last elapsed second. |
 | haproxy_process_max_frontend_ssl_key_rate  | Maximum observed frontend 
SSL Key computation per second. |
-| haproxy_process_frontent_ssl_reuse | SSL session reuse ratio 
(percent).|
+| haproxy_process_frontend_ssl_reuse | SSL session reuse ratio 
(percent).|
 | haproxy_process_current_backend_ssl_key_rate   | Current backend SSL Key 
computation per second over last elapsed second.  |
 | haproxy_process_max_backend_ssl_key_rate   | Maximum observed backend 
SSL Key computation per second.  |
 | haproxy_process_ssl_cache_lookups_total| Total number of SSL session 
cache lookups.|
diff --git a/contrib/prometheus-exporter/service-prometheus.c 
b/contrib/prometheus-exporter/service-prometheus.c
index 952558c70..009e817ae 100644
--- a/contrib/prometheus-exporter/service-prometheus.c
+++ b/contrib/prometheus-exporter/service-prometheus.c
@@ -485,7 +485,7 @@ const struct ist promex_inf_metric_names[INF_TOTAL_FIELDS] 
= {
[INF_MAX_SSL_RATE]   = IST("max_ssl_rate"),
[INF_SSL_FRONTEND_KEY_RATE]  = 
IST("current_frontend_ssl_key_rate"),
[INF_SSL_FRONTEND_MAX_KEY_RATE]  = IST("max_frontend_ssl_key_rate"),
-   [INF_SSL_FRONTEND_SESSION_REUSE_PCT] = IST("frontent_ssl_reuse"),
+   [INF_SSL_FRONTEND_SESSION_REUSE_PCT] = IST("frontend_ssl_reuse"),
[INF_SSL_BACKEND_KEY_RATE]   = 
IST("current_backend_ssl_key_rate"),
[INF_SSL_BACKEND_MAX_KEY_RATE]   = IST("max_backend_ssl_key_rate"),
[INF_SSL_CACHE_LOOKUPS]  = IST("ssl_cache_lookups_total"),
-- 
2.27.0

Re: [PATCH] cleanup coverity findging (make it silent)

[Илья Шипицин]

well, I do not have an idea why


extchk_setenv(check, EXTCHK_HAPROXY_SERVER_ADDR, check->argv[3]);

is used instead of

EXTCHK_SETENV(check, EXTCHK_HAPROXY_SERVER_ADDR, check->argv[3], err);


it means, some environment variables are set in "best effort" mode, i.e.
error is ignored. is it bad ? I'm not sure.


code comes from

https://github.com/haproxy/haproxy/commit/61cc85223098a962616ececa2d6bdd7809c37fe3

Christopher, do you know why we ignore exit status here ?


вт, 26 мая 2020 г. в 19:59, Илья Шипицин :

>
>
> вт, 26 мая 2020 г. в 12:02, Willy Tarreau :
>
>> Hi Ilya,
>>
>> On Sat, May 23, 2020 at 03:47:58PM +0500,  ??? wrote:
>> > From: Ilya Shipitsin 
>> > Date: Sat, 23 May 2020 15:35:36 +0500
>> > Subject: [PATCH] CLEANUP: src/checks.c: ignore return value using
>> DISGUISE(..)
>> >
>> > we do not want to check status of extchk_setenv, but static analyzsers
>> > like Coverity are not happy about it. Let calm coverity down.
>>
>> Are you really sure we don't want to check them ? I'm seeing that
>> prepare_external_check() uses EXTCHK_SETENV() to purposely add checks
>> there, so it's unclear to me why we want to silently fail here. Maybe
>> the calls should instead be changed to have a check and a jump to an
>> error label doing the exit().
>>
>> I don't know if anyone has an opinion on this, I'm not using external
>> checks :-/
>>
>
> well, I meant to keep current behaviour, but also silence coverity warning.
>
> ok, we can investigate and discuss would it be better to change current
> behaviour or to keep it.
>
>
>>
>> Willy
>>
>

Re: [PATCH] cleanup coverity findging (make it silent)

[Илья Шипицин]

вт, 26 мая 2020 г. в 12:02, Willy Tarreau :

> Hi Ilya,
>
> On Sat, May 23, 2020 at 03:47:58PM +0500,  ??? wrote:
> > From: Ilya Shipitsin 
> > Date: Sat, 23 May 2020 15:35:36 +0500
> > Subject: [PATCH] CLEANUP: src/checks.c: ignore return value using
> DISGUISE(..)
> >
> > we do not want to check status of extchk_setenv, but static analyzsers
> > like Coverity are not happy about it. Let calm coverity down.
>
> Are you really sure we don't want to check them ? I'm seeing that
> prepare_external_check() uses EXTCHK_SETENV() to purposely add checks
> there, so it's unclear to me why we want to silently fail here. Maybe
> the calls should instead be changed to have a check and a jump to an
> error label doing the exit().
>
> I don't know if anyone has an opinion on this, I'm not using external
> checks :-/
>

well, I meant to keep current behaviour, but also silence coverity warning.

ok, we can investigate and discuss would it be better to change current
behaviour or to keep it.


>
> Willy
>

Re: [PATCH] cleanup coverity findging (make it silent)

[Willy Tarreau]

Hi Ilya,

On Sat, May 23, 2020 at 03:47:58PM +0500,  ??? wrote:
> From: Ilya Shipitsin 
> Date: Sat, 23 May 2020 15:35:36 +0500
> Subject: [PATCH] CLEANUP: src/checks.c: ignore return value using DISGUISE(..)
> 
> we do not want to check status of extchk_setenv, but static analyzsers
> like Coverity are not happy about it. Let calm coverity down.

Are you really sure we don't want to check them ? I'm seeing that
prepare_external_check() uses EXTCHK_SETENV() to purposely add checks
there, so it's unclear to me why we want to silently fail here. Maybe
the calls should instead be changed to have a check and a jump to an
error label doing the exit().

I don't know if anyone has an opinion on this, I'm not using external
checks :-/

Willy