Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

slightly simplify and clean up the qmail-send job helper functions #213

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

DerDakon
Copy link
Member

No description provided.

Copy link
Contributor

@mbhangui mbhangui left a comment

Choose a reason for hiding this comment

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

This is good. By returning the job number in job_avail() and using that in job_open() one entire for(;;) loop has been eliminated.

@schmonz schmonz added the bug Something isn't working label Apr 2, 2021
@leahneukirchen
Copy link
Contributor

pass_selprep needs adjustment for the new return value of job_avail (

if (job_avail())
), else lgtm but I'm not super familiar with this code.

@schmonz
Copy link
Member

schmonz commented Aug 24, 2021

My "Legacy Open Source Fridays" ensemble-programming session will be submitting a PR shortly (probably this Friday) to add tests around the existing behaviors of qmail-send:job_*. If that PR looks mergeable, we should merge it first and then rebase this one, adjusting the tests as needed.

@DerDakon
Copy link
Member Author

I would welcome that. Since I plan to make them static the file itself should be included, not just linked to.

job_avail() already knew which was the next available job slot. job_open() would
scan for the same number again. In case there would be no job free by then it
returned -1, which was never checked, but used as an array index.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working development
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants