Skip to content

core: use distinct names for JobControlRecordPrivate#1307

Merged
pstorz merged 2 commits intobareos:masterfrom
arogge:dev/arogge/master/split-jcr-private
Nov 9, 2022
Merged

core: use distinct names for JobControlRecordPrivate#1307
pstorz merged 2 commits intobareos:masterfrom
arogge:dev/arogge/master/split-jcr-private

Conversation

@arogge
Copy link
Member

@arogge arogge commented Nov 8, 2022

When we moved the daemon-specific functionality of the JobControlRecord into JobControlRecordPrivate, we essentially introduced three different types that were all named JobControlRecordPrivate - one for the Director, one for the Storage Daemon and one for the File Daemon. This could lead to ODR violations when you tried to link parts of the daemons together.
In fact, that implementation already behaved like a union of the three different types (i.e. requiring per-implementation initialisation and teardown, undefined behaviour when accessing via the wrong type, etc.).

This patch renames the daemon's individual JobControlRecordPrivate to DirectorJcrImpl, StoredJcrImpl and FiledJcrImpl. The implementation-pointer was changed to a union of dir_impl, sd_impl and fd_impl. With this change applied you can now build programs that use two or more of the JCR types.

Thank you for contributing to the Bareos Project!

Please check

  • Short description and the purpose of this PR is present above this paragraph
  • Your name is present in the AUTHORS file (optional)

If you have any questions or problems, please give a comment in the PR.

Helpful documentation and best practices

Checklist for the reviewer of the PR (will be processed by the Bareos team)

General
  • PR name is meaningful
  • Purpose of the PR is understood
  • Commit descriptions are understandable and well formatted
  • Check backport line
  • Is the PR title usable as CHANGELOG entry?
  • Separate commit for CHANGELOG.md ("update CHANGELOG.md"). The PR number is correct.
Source code quality
  • Source code changes are understandable
  • Variable and function names are meaningful
  • Code comments are correct (logically and spelling)
  • Required documentation changes are present and part of the PR
  • bareos-check-sources --since-merge does not report any problems

@arogge arogge requested a review from pstorz November 8, 2022 18:18
@arogge arogge force-pushed the dev/arogge/master/split-jcr-private branch from c3e3db0 to e8c77bc Compare November 8, 2022 18:19
Copy link
Member

@pstorz pstorz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good work!

@pstorz pstorz changed the title Use distinct names for JobControlRecordPrivate core: use distinct names for JobControlRecordPrivate Nov 9, 2022
arogge and others added 2 commits November 9, 2022 17:29
When we moved the daemon-specific functionality of the JobControlRecord
into JobControlRecordPrivate, we essentially introduced three different
types that were all named JobControlRecordPrivate - one for the
Director, one for the Storage Daemon and one for the File Daemon. This
could lead to ODR violations when you tried to link parts of the daemons
together.
In fact, that implementation already behaved like a union of the three
different types (i.e. requiring per-implementation initialisation and
teardown, undefined behaviour when accessing via the wrong type, etc.).

This patch renames the daemon's individual JobControlRecordPrivate to
DirectorJcrImpl, StoredJcrImpl and FiledJcrImpl. The impl-pointer was
changed to a union of dir_impl, sd_impl and fd_impl.
With this change applied you can now build programs that use two or more
of the JCR types.
@pstorz pstorz force-pushed the dev/arogge/master/split-jcr-private branch from d3bb536 to 9fd878e Compare November 9, 2022 16:29
@pstorz pstorz merged commit 6032bd7 into bareos:master Nov 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants