Hi guys, On Thu, Sep 26, 2019 at 04:35:31PM +0200, Tim Düsterhus wrote: > Adis, > > Am 26.09.19 um 16:01 schrieb Adis Nezirovic: > > While I agree that using zero based array indexing is a mistake (wearing > > my Lua hat), I don't think your proposal will make situation any better. > > Actually ot might hurt existing Lua code in unforeseen ways. > > Yes, I understand that it breaks backwards compatibility. That's why I > added the cover letter with the rationale of duplicating the first value > instead of making the 0-index a nil.
To be honest I'm totally ignorant of this code area and it's even not very clear to me (even after reading the commit messages) what is the extent of this change nor what this headers array corresponds to. However what I can say is that when implementing a transition towards a final solution by having an intermediate solution (like you're proposing by keeping index 0), it's important to have a plan to get rid of the intermediate solution at some point. Here, by duplicating entries I'm not seeing any way to properly cover this final transition, nor to safely avoid breakage of existing applications. Thus it will turn an existing problem into two other ones. I could instead propose two solutions : - either we knowingly break existing code for 2.1+ by changing the array numbering to match Lua standards ; those migrating from 2.0 to 2.1 or 2.2 will have to modify their code. We can explain them how to make portable code by checking the haproxy version from within Lua if required; - or we decide to rename this "headers" field on the long term (if it's the only one, which is really the part I'm ignoring) and create a new one ("hdrs" ?) numbered from 1. We can thus keep the two in parallel till 2.2 included so that at least one LTS version covers both the old and new API, and in 2.3 we remove headers. If we can emit an API warning when "headers" is first met, it's even better. I just don't know how we can make one structure field alias another one, I can understand that it's not just a matter of setting +1 on a pointer, but I guess that we have hooks behind the curtains which intercept this "headers" thing. There may also be other solutions, but definitely an API change needs to pass through something visible so that affected code at some point has to be fixed if we want to get rid of a past mistake. Willy