-
Notifications
You must be signed in to change notification settings - Fork 185
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
CFE-4053: Fixed bug where default:sys.fqhost contained many spaces #5531
Conversation
Fixed bug where `default:sys.fqhost` contained many spaces when domain is set in body common control. Ticket: CFE-4053 Changelog: Commit Signed-off-by: Lars Erik Wik <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me otherwise.
if => strcmp( "$(sys.fqhost)", "$(sys.uqhost).cfengine.com" ); | ||
|
||
"$(this.promise_filename) FAIL" | ||
unless => strcmp( "$(sys.fqhost)", "$(sys.uqhost).cfengine.com" ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't expect this to work on my machine. And many other machines...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because I don't think my machine has $(sys.uqhost).cfengine.com
as its FQDN. Or are we ensuring that somewhere?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Aren't we doing that with domain => "cfengine.com"
in body common control
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh! I guess we are! I didn't see that. Or rather, I didn't even know it was possible. 😅
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks reasonable to me.
EvalContextVariablePutSpecial(ctx, SPECIAL_SCOPE_SYS, "domain", | ||
VDOMAIN, CF_DATA_TYPE_STRING, | ||
"source=agent"); | ||
EvalContextClassPutHard(ctx, VDOMAIN, "source=agent"); | ||
|
||
int ret = snprintf(VFQNAME, CF_MAXVARSIZE, "%s.%s", VUQNAME, VDOMAIN); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@larsewi In this case, the diff is rather big, but the important difference is the change from %511s.%511s
to %s.%s
. I would've explained this in the commit (as you did in the ticket).
Fixed bug where
default:sys.fqhost
contained many spaces when domain is set in body common control.Ticket: CFE-4053
Changelog: Commit
Signed-off-by: Lars Erik Wik [email protected]