Re: [PATCH 2/3] BUG/MINOR: select: remove uneeded free in thread init
On Wed, May 13, 2020 at 11:50:50AM +0200, Willy Tarreau wrote: > It's not a bug, it's just a way to ensure a common error path undoes > everything that was done before reaching it. If another operation is > added after the DIR_WR malloc, it will only have to add its own undo > operation to the fail label instead of having to figure which one was > the last to suceed and whether or not it needs to be addressed. > > Thus I'd rather not change it since it does not have any impact and > makes the code more future-proof. understood, thanks for the feedback. -- William
Re: [PATCH 2/3] BUG/MINOR: select: remove uneeded free in thread init
On Mon, May 11, 2020 at 03:20:04PM +0200, William Dauchy wrote: > we wrongly free `tmp_evts[DIR_RD]` in error case. It's not a bug, it's just a way to ensure a common error path undoes everything that was done before reaching it. If another operation is added after the DIR_WR malloc, it will only have to add its own undo operation to the fail label instead of having to figure which one was the last to suceed and whether or not it needs to be addressed. Thus I'd rather not change it since it does not have any impact and makes the code more future-proof. Thanks, Willy
[PATCH 2/3] BUG/MINOR: select: remove uneeded free in thread init
we wrongly free `tmp_evts[DIR_RD]` in error case.
it could be backported from v1.8 to v2.1.
Fixes: d4604adeaa8c ("MAJOR: threads/fd: Make fd stuffs thread-safe")
Signed-off-by: William Dauchy
---
src/ev_select.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/src/ev_select.c b/src/ev_select.c
index acfdbb94a..167bb7eb7 100644
--- a/src/ev_select.c
+++ b/src/ev_select.c
@@ -226,13 +226,13 @@ static int init_select_per_thread()
fd_set_bytes = sizeof(fd_set) * (global.maxsock + FD_SETSIZE - 1) /
FD_SETSIZE;
if ((tmp_evts[DIR_RD] = (fd_set *)calloc(1, fd_set_bytes)) == NULL)
- goto fail;
+ goto fail_rd;
if ((tmp_evts[DIR_WR] = (fd_set *)calloc(1, fd_set_bytes)) == NULL)
- goto fail;
+ goto fail_wr;
return 1;
- fail:
+ fail_wr:
free(tmp_evts[DIR_RD]);
- free(tmp_evts[DIR_WR]);
+ fail_rd:
return 0;
}
--
2.26.2
