Re: [PATCH 0/6] Probably final Coccinelle Cleanup
On Mon, Nov 08, 2021 at 12:53:00PM +0100, Tim Düsterhus wrote: > Willy, > > On 11/8/21 11:43 AM, Willy Tarreau wrote: > > > You're totally right. Not only it is redundant, but it is wrong (which > > > is why it is redundant). By being called strncat() one would hope that > > > it never copies more than the source, but here it ignores the source's > > > size and always takes the len argument. I suggest that we replace it > > > with chunk_memcat() everywhere and remove it to avoid any issue. At > > > first glance for now this is harmless, fortunately, but for how long? > > > > I'm going to do it, and have already applied your 6 first patches here. > > Thanks for handling this. > > Can you please revert b9656e48377a9e5359494bce6a413a9915c8f74b then? This > Coccinelle patch is no longer needed / correct now that the function is > gone. OK done. Willy
Re: [PATCH 0/6] Probably final Coccinelle Cleanup
Willy, On 11/8/21 11:43 AM, Willy Tarreau wrote: You're totally right. Not only it is redundant, but it is wrong (which is why it is redundant). By being called strncat() one would hope that it never copies more than the source, but here it ignores the source's size and always takes the len argument. I suggest that we replace it with chunk_memcat() everywhere and remove it to avoid any issue. At first glance for now this is harmless, fortunately, but for how long? I'm going to do it, and have already applied your 6 first patches here. Thanks for handling this. Can you please revert b9656e48377a9e5359494bce6a413a9915c8f74b then? This Coccinelle patch is no longer needed / correct now that the function is gone. Best regards Tim Düsterhus
Re: [PATCH 0/6] Probably final Coccinelle Cleanup
On Mon, Nov 08, 2021 at 11:41:52AM +0100, Willy Tarreau wrote: > Hi Tim, > > On Mon, Nov 08, 2021 at 09:04:59AM +0100, Tim Duesterhus wrote: > > Hi Willy, > > > > find my (probably :-) ) final CLEANUP series for 2.5. > > > > Regarding the final patch: > > > > 'chunk_strncat()' appears to be completely redundant, it simply passes > > through > > the arguments and even takes an int instead of a size_t. Should it be > > removed? > > You're totally right. Not only it is redundant, but it is wrong (which > is why it is redundant). By being called strncat() one would hope that > it never copies more than the source, but here it ignores the source's > size and always takes the len argument. I suggest that we replace it > with chunk_memcat() everywhere and remove it to avoid any issue. At > first glance for now this is harmless, fortunately, but for how long? I'm going to do it, and have already applied your 6 first patches here. Willy
Re: [PATCH 0/6] Probably final Coccinelle Cleanup
Hi Tim, On Mon, Nov 08, 2021 at 09:04:59AM +0100, Tim Duesterhus wrote: > Hi Willy, > > find my (probably :-) ) final CLEANUP series for 2.5. > > Regarding the final patch: > > 'chunk_strncat()' appears to be completely redundant, it simply passes through > the arguments and even takes an int instead of a size_t. Should it be removed? You're totally right. Not only it is redundant, but it is wrong (which is why it is redundant). By being called strncat() one would hope that it never copies more than the source, but here it ignores the source's size and always takes the len argument. I suggest that we replace it with chunk_memcat() everywhere and remove it to avoid any issue. At first glance for now this is harmless, fortunately, but for how long? Willy
[PATCH 0/6] Probably final Coccinelle Cleanup
Hi Willy, find my (probably :-) ) final CLEANUP series for 2.5. Regarding the final patch: 'chunk_strncat()' appears to be completely redundant, it simply passes through the arguments and even takes an int instead of a size_t. Should it be removed? Best regards Tim Düsterhus Tim Duesterhus (6): DEV: coccinelle: Add rule to use `isttrim()` where possible CLEANUP: Apply ist.cocci DEV: coccinelle: Add rule to use `chunk_istcat()` instead of `chunk_memcat()` DEV: coccinelle: Add rule to use `chunk_istcat()` instead of `chunk_strncat()` CLEANUP: Apply ist.cocci CLEANUP: chunk: Remove duplicated chunk_Xcat implementation dev/coccinelle/ist.cocci | 24 +++ include/haproxy/chunk.h | 41 +--- src/cache.c | 5 ++--- src/flt_trace.c | 3 +-- src/hlua.c | 6 ++ src/http_ana.c | 3 +-- src/http_fetch.c | 2 +- src/http_htx.c | 4 ++-- src/log.c| 6 ++ src/mux_fcgi.c | 10 +- src/tcpcheck.c | 4 ++-- 11 files changed, 55 insertions(+), 53 deletions(-) -- 2.33.1
