Hi Tim,
On Sat, Mar 05, 2022 at 12:52:39AM +0100, Tim Duesterhus wrote:
> Willy,
> Christopher,
>
> find a series that converts a few members of `struct proxy` into ists. All
> of them have already been converted into ists when operating on them, so
> directly storing them as ists makes that code cleaner.
Now applied, thank you for doing this, it's always good to see such
remains of old code diminish over time. If you're interested in chasing
other ones, I'm seeing that the following commands seem to yield likely
candidates:
$ git grep -B1 '_len;' include
> One drawback is that `struct proxy` grows by 16 bytes. It might or might not
> be necessary to reorder the struct members to keep it efficient.
Sure but 16 bytes for such a huge struct is not much, and indeed there
are a few holes that we may repack later:
$ pahole -C proxy ./haproxy
/* size: 6944, cachelines: 109, members: 133 */
/* sum members: 6924, holes: 5, sum holes: 20 */
/* paddings: 1, sum paddings: 4 */
/* last cacheline: 32 bytes */
> The `server_id_hdr_name` one is tagged MEDIUM, because that required some
> non-trivial changes in the FCGI implementation. As I've needed to modify
> that code anyway, I've also added two additional CLEANUP commits.
For me these looked good.
> As for the second CLEANUP commit: If one of you knows how to fix the
> Coccinelle
> patch to detect that specific pattern, I'd appreciate if you could make the
> necessary changes to ist.cocci. Unfortunately my Coccinelle skills are not
> good enough.
I've already faced this situation where it didn't find fields of a given
type inside a structure, and I stopped searching when I figured that by
the time I would finally find, I could have replaced 10 times the 3
occurrences I needed. I essentially use Coccinelle as a helpful tool to
save me time, and I can't resolve myself to spend more time trying to
write unmaintainable scripts that will never be reused. A good source of
inspiration are the scripts in the linux kernel, but those are often of
an extreme level of complexity and mix python scripting within the patch,
resulting in me not understanding anything anymore. But some of them are
still human-readable or at least show the syntax hacks you're looking for.
> I've tested that HAProxy compiles and that the existing reg-tests pass, but
> I didn't specifically test FCGI (and there are no exiting reg-tests for that).
There are indeed no regtest because we don't have an fcgi server either
in vtest nor in haproxy.
> So please carefully check the patches for dumb mistakes.
For me that was OK so I merged them. If we later find a regression, we'll
both be co-responsible :-)
Thanks,
Willy
