Skip to content

Add support for default stratum split #137

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

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

Conversation

rleed
Copy link

@rleed rleed commented Jul 17, 2025

The intention of this PR is to add support for a default stratum split. If no tilde is present in the stratum username, one is automatically added at the end of the username it behaves as with a blank modname. If a blank modname exists in the configuration, it will then serve as the default stratum split.

Copy link
Contributor

@luke-jr luke-jr left a comment

Choose a reason for hiding this comment

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

Seems like it would be better to just pass modname_len as 0?

@luke-jr
Copy link
Contributor

luke-jr commented Jul 17, 2025

Also, this means the username mod stuff is always enabled... Probably should have a bool on datum_config that gets set if "" is being used, so we're not wasting CPU on setups that don't use the feature.

@rleed
Copy link
Author

rleed commented Jul 17, 2025

Also, this means the username mod stuff is always enabled...

Doesn't if (datum_config.stratum_username_mod) disable it if it's not configured?

@rleed
Copy link
Author

rleed commented Jul 17, 2025

Seems like it would be better to just pass modname_len as 0?

// CAUTION: modname MUST be part of username_s following a tilde made me wary. The datum_stratum_mod_username function indexes the tilde character without checking that it exists first, which I think can't be assumed when just passing modname_len as 0.

@rleed
Copy link
Author

rleed commented Jul 18, 2025

Also, this means the username mod stuff is always enabled...

Doesn't if (datum_config.stratum_username_mod) disable it if it's not configured?

Ok, now I got it. I made the changes to skip the new logic if there is no default mod in the configuration.

@rleed rleed requested a review from luke-jr July 18, 2025 11:37
@rleed
Copy link
Author

rleed commented Jul 19, 2025

Seems like it would be better to just pass modname_len as 0?

// CAUTION: modname MUST be part of username_s following a tilde made me wary. The datum_stratum_mod_username function indexes the tilde character without checking that it exists first, which I think can't be assumed when just passing modname_len as 0.

I've now modified the existing code to remove the caution above and pass a len of 0.

@luke-jr luke-jr added the enhancement New feature or request label Jul 22, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants