Re: [PR] hpack-tbl-t.h uses VAR_ARRAY and requires compiler.h to be included
No Problem at all. Feel free :) On 2020-12-21 12:59, Willy Tarreau wrote: On Mon, Dec 21, 2020 at 12:20:36PM +0100, Christian Ruppert wrote: > 2) we include from this file so that it becomes > consistent > with everything else ; > > 3) we add the ifdef VAR_ARRAY directly into the file so that it > continues > not to depend on anything and can be directly imported into other > projects as needed. > > I guess I prefer the 3rd option here as it's extremely cheap and will > keep external build setups very straightforward. What do you think ? > > Thanks! > Willy 2. and 3. sounds good. 3. however seems to be the best solution, indeed. OK, do you mind if I just modify your patch and commit message according to this ? Or do you prefer to send a new one ? I'm asking because while I usually have no problem modifying patches or commit messages, I don't do it when they're signed. Thanks, Willy -- Regards, Christian Ruppert
Re: [PR] hpack-tbl-t.h uses VAR_ARRAY and requires compiler.h to be included
On Mon, Dec 21, 2020 at 12:20:36PM +0100, Christian Ruppert wrote: > > 2) we include from this file so that it becomes > > consistent > > with everything else ; > > > > 3) we add the ifdef VAR_ARRAY directly into the file so that it > > continues > > not to depend on anything and can be directly imported into other > > projects as needed. > > > > I guess I prefer the 3rd option here as it's extremely cheap and will > > keep external build setups very straightforward. What do you think ? > > > > Thanks! > > Willy > > 2. and 3. sounds good. 3. however seems to be the best solution, indeed. OK, do you mind if I just modify your patch and commit message according to this ? Or do you prefer to send a new one ? I'm asking because while I usually have no problem modifying patches or commit messages, I don't do it when they're signed. Thanks, Willy
Re: [PR] hpack-tbl-t.h uses VAR_ARRAY and requires compiler.h to be included
Hey Willy, On 2020-12-21 11:36, Willy Tarreau wrote: Hi, On Sun, Dec 20, 2020 at 12:58:52PM +0500, ??? wrote: ping :) Oh I completely missed this one in the noise it seems! I'm sorry. No problem! :) > Author: Christian Ruppert > Number of patches: 1 > > This is an automated relay of the Github pull request: >hpack-tbl-t.h uses VAR_ARRAY and requires compiler.h to be included I initially tried hard not to put haproxy-specific dependencies in these protocol-specific parts so that they could easily be reused by other projects if needed (hence the relaxed MIT license). But I guess adding compiler.h is not that big of a deal. However I disagree with including it from the same directory with double-quotes, as we try to keep our includes more or less ordered with certain dependencies. Thus Christian, I can offer 3 possibilities here, I don't know which one best suits your use case: 1) we include from this file. It will best follow the current practices all over the code, but may or may not work for your use case depending how you include the file; 2) we include from this file so that it becomes consistent with everything else ; 3) we add the ifdef VAR_ARRAY directly into the file so that it continues not to depend on anything and can be directly imported into other projects as needed. I guess I prefer the 3rd option here as it's extremely cheap and will keep external build setups very straightforward. What do you think ? Thanks! Willy 2. and 3. sounds good. 3. however seems to be the best solution, indeed. -- Regards, Christian Ruppert
Re: [PR] hpack-tbl-t.h uses VAR_ARRAY and requires compiler.h to be included
Hi, On Sun, Dec 20, 2020 at 12:58:52PM +0500, ??? wrote: > ping :) Oh I completely missed this one in the noise it seems! I'm sorry. > > Author: Christian Ruppert > > Number of patches: 1 > > > > This is an automated relay of the Github pull request: > >hpack-tbl-t.h uses VAR_ARRAY and requires compiler.h to be included I initially tried hard not to put haproxy-specific dependencies in these protocol-specific parts so that they could easily be reused by other projects if needed (hence the relaxed MIT license). But I guess adding compiler.h is not that big of a deal. However I disagree with including it from the same directory with double-quotes, as we try to keep our includes more or less ordered with certain dependencies. Thus Christian, I can offer 3 possibilities here, I don't know which one best suits your use case: 1) we include from this file. It will best follow the current practices all over the code, but may or may not work for your use case depending how you include the file; 2) we include from this file so that it becomes consistent with everything else ; 3) we add the ifdef VAR_ARRAY directly into the file so that it continues not to depend on anything and can be directly imported into other projects as needed. I guess I prefer the 3rd option here as it's extremely cheap and will keep external build setups very straightforward. What do you think ? Thanks! Willy
Re: [PR] hpack-tbl-t.h uses VAR_ARRAY and requires compiler.h to be included
ping :) пн, 9 нояб. 2020 г. в 14:26, PR Bot : > Dear list! > > Author: Christian Ruppert > Number of patches: 1 > > This is an automated relay of the Github pull request: >hpack-tbl-t.h uses VAR_ARRAY and requires compiler.h to be included > > Patch title(s): >hpack-tbl-t.h uses VAR_ARRAY and requires compiler.h to be included > > Link: >https://github.com/haproxy/haproxy/pull/942 > > Edit locally: >wget https://github.com/haproxy/haproxy/pull/942.patch && vi 942.patch > > Apply locally: >curl https://github.com/haproxy/haproxy/pull/942.patch | git am - > > Description: >This fixes building hpack from contrib, which failed because of the >undeclared VAR_ARRAY: > >make -C contrib/hpack >... >cc >-O2 -Wall -g -I../../include -fwrapv -fno-strict-aliasing -c -o gen- >enc.o gen-enc.c >In file included from gen-enc.c:18: >../../include/haproxy/hpack-tbl-t.h:105:23: error: 'VAR_ARRAY' >undeclared here (not in a function) > 105 | struct hpack_dte >dte[VAR_ARRAY]; /* dynamic table entries */ >... > >Signed- >off-by: Christian Ruppert > > Instructions: >This github pull request will be closed automatically; patch should be >reviewed on the haproxy mailing list (haproxy@formilux.org). Everyone > is >invited to comment, even the patch's author. Please keep the author and >list CCed in replies. Please note that in absence of any response this >pull request will be lost. > >
[PR] hpack-tbl-t.h uses VAR_ARRAY and requires compiler.h to be included
Dear list! Author: Christian Ruppert Number of patches: 1 This is an automated relay of the Github pull request: hpack-tbl-t.h uses VAR_ARRAY and requires compiler.h to be included Patch title(s): hpack-tbl-t.h uses VAR_ARRAY and requires compiler.h to be included Link: https://github.com/haproxy/haproxy/pull/942 Edit locally: wget https://github.com/haproxy/haproxy/pull/942.patch && vi 942.patch Apply locally: curl https://github.com/haproxy/haproxy/pull/942.patch | git am - Description: This fixes building hpack from contrib, which failed because of the undeclared VAR_ARRAY: make -C contrib/hpack ... cc -O2 -Wall -g -I../../include -fwrapv -fno-strict-aliasing -c -o gen- enc.o gen-enc.c In file included from gen-enc.c:18: ../../include/haproxy/hpack-tbl-t.h:105:23: error: 'VAR_ARRAY' undeclared here (not in a function) 105 | struct hpack_dte dte[VAR_ARRAY]; /* dynamic table entries */ ... Signed- off-by: Christian Ruppert Instructions: This github pull request will be closed automatically; patch should be reviewed on the haproxy mailing list (haproxy@formilux.org). Everyone is invited to comment, even the patch's author. Please keep the author and list CCed in replies. Please note that in absence of any response this pull request will be lost.
