Skip to content

Enable default max-cache-size tweaks at build time

Michał Kępień requested to merge 2075-enable-default-max-cache-size-tweaks-at-build-time into main

Overriding the default value of max-cache-size turned out to be surprisingly irritating to implement. This MR is the best thing I have so far, but I would like to request feedback at this stage, to make sure the approach proposed here is the one we should stick with.

I first tried to make this a -T option to named that would be passed by default in bin/tests/system/start.pl, but that approach:

  • complicates the source code: we need this option to only override the implicit default in bin/named/config.c, not anything explicitly set in named.conf; this necessitates processing the value twice, each time with different configuration maps (maps vs. optionmaps), which in turn requires the code processing that value to first be extracted to a separate function to avoid code duplication; even with that done, the resulting diff is still pretty ugly given the problem at hand,

  • does not affect all GitLab CI builds by default, which means the extra -T option would need to be included in all named.args files now and in the future (i.e. this makes the problem prone to being glossed over with time).

Given the above drawbacks, I settled for using a preprocessor macro instead. The code change is simple enough and should resolve the problem for good in the future, but it has other issues:

  • we need to employ the stringizing operator (a configure option would be overkill and setting the value of a preprocessor macro to something containing quotes causes all hell to break loose in one place or another in the build process),

  • clang-format does not like it - and to make things worse, the changes it proposes break the solution :-) it treats the percent sign in the default value of the macro as the modulo operator and adding a space before it breaks named.conf syntax.

Interestingly enough, clang-format seems to flag other, pre-existing issues when run for this branch. I do not understand why it does that, i.e. why it is happy with the current main code as it is.

Which of these two approaches do you think is lesser evil? Or maybe you have a better idea for addressing this problem?

Closes #2075 (closed)

Merge request reports