director: fix crash in status scheduler when client is not set

[pstorz]

Thank you for contributing to the Bareos Project!

The status scheduler command did not check the job->client pointer before dereferencing. This is fixed with this PR.

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

• Git Workflow
• Automatic Sourcecode Formatting
• Check your commit messages

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
• Separate commit for this PR in the CHANGELOG.md, PR number referenced is same
• Commit descriptions are understandable and well formatted

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
• git status should not report modifications in the source tree after building and testing

[arogge]

While this patch fixes the crash, it will make status scheduler behave in a way that I would not expect. So it is probably not a good idea to do it this way.

Maybe we could improve the job object to have a function is_client_enabled() that could handle it. That function could look like this:

bool is_client_enabled() {
  return client != nullptr && client->enabled;
}

This would return false if client is set and not enabled, but true in every other case including client not being set.

[arogge]

Just tried with your changes.
Configured a copy-job with a disabled client and ran status scheduler job=copy.
Threw the direcotor in an infinite loop eating CPU, so something is obviously still broken.

[arogge]

in ua_status.cc we have lines 658 to 716 with 7 empty or comment lines.
That's a net amount of 51 lines of code.

These contain:

• 16 branches (14 ifs, 2 elses)
• 3 loops (1 while, 2 foreach_res)
• 9 jumps (3 gotos, 5 continues, 1 break)

So there is a loop, branch or some other kind of jump every 2 lines.

What I'm trying to say is: I would love to review the code, but I need to understand it.
Don't you think this should be refactored completely so we can understand what is going on without drawing a complex diagram?

The commits with the changes do not exist anymore