Skip to content
GitLab
Projects Groups Topics Snippets
  • /
  • Help
    • Help
    • Support
    • Community forum
    • Submit feedback
    • Contribute to GitLab
  • Register
  • Sign in
  • BIND BIND
  • Project information
    • Project information
    • Activity
    • Labels
    • Members
  • Repository
    • Repository
    • Files
    • Commits
    • Branches
    • Tags
    • Contributor statistics
    • Graph
    • Compare revisions
  • Issues 633
    • Issues 633
    • List
    • Boards
    • Service Desk
    • Milestones
  • Merge requests 87
    • Merge requests 87
  • CI/CD
    • CI/CD
    • Pipelines
    • Jobs
    • Artifacts
    • Schedules
  • Deployments
    • Deployments
    • Environments
    • Releases
  • Packages and registries
    • Packages and registries
    • Container Registry
  • Monitor
    • Monitor
    • Incidents
  • Analytics
    • Analytics
    • Value stream
    • CI/CD
    • Repository
  • Wiki
    • Wiki
  • Snippets
    • Snippets
  • Activity
  • Graph
  • Create a new issue
  • Jobs
  • Commits
  • Issue Boards
Collapse sidebar
  • ISC Open Source ProjectsISC Open Source Projects
  • BINDBIND
  • Merge requests
  • !3962

Enable default max-cache-size tweaks at build time

  • Review changes

  • Download
  • Patches
  • Plain diff
Closed Michał Kępień requested to merge 2075-enable-default-max-cache-size-tweaks-at-build-time into main Aug 14, 2020
  • Overview 6
  • Commits 2
  • Pipelines 1
  • Changes 2

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)

Assignee
Assign to
Reviewers
Request review from
Time tracking
Source branch: 2075-enable-default-max-cache-size-tweaks-at-build-time