Skip to content

Conversation

@nickanderson
Copy link
Member

Added test for copy_from which already respects move_obstructions. Foundation for testing other files promise behavior with move_obstructions and symlinks.

Ticket: CFE-4591

@nickanderson
Copy link
Member Author

 ./10_files/move_obstructions-promiser-is-symlink.cf Pass

@nickanderson
Copy link
Member Author

nickanderson commented Sep 28, 2025

As expected:

./10_files/move_obstructions-promiser-is-symlink.cf FAIL (UNEXPECTED FAILURE)

@nickanderson
Copy link
Member Author

./10_files/move_obstructions-promiser-is-symlink.cf Pass

return false;
}

const mode_t st_mode = sb->st_mode;

Check notice

Code scanning / CodeQL

Pointer argument is dereferenced without checking for NULL Note

Parameter sb in HandleFileObstruction() is dereferenced without an explicit null-check
Copy link
Member Author

Choose a reason for hiding this comment

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

@larsewi i had done that safe_attr and safe_sb to avoid this check

Copy link
Contributor

Choose a reason for hiding this comment

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

I thought so. But it should not be an issue anymore with the assert's.

Assigning them to a local variable does not really solve the issue. Although, you had some if-statements that in fact does solve it. However, these variables should not be NULL, so an assert be sufficient.

Copy link
Member Author

Choose a reason for hiding this comment

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

@larsewi I'm sorry, it's been three weeks and I no longer understand this context. You said you would push some commit on top monday. Sounds great to me I can squash it all together at the end.

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm very sorry @nickanderson. I see that you have actually done exactly what I asked for. Looks great 🚀

Copy link
Contributor

@craigcomstock craigcomstock left a comment

Choose a reason for hiding this comment

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

Looks good. Would be ready to go after squashing a few commits and passing tests.

Copy link
Contributor

@larsewi larsewi left a comment

Choose a reason for hiding this comment

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

Great 🚀 Please squash and merge!

@basvandervlies
Copy link
Contributor

thanks!

- Added test for copy_from and move_obstructions
- Added test and support for content attribute targeting symlink with move_obstructions
- Added test and support for move_obstructions for symlinks targeted by inline_mustache
- Added tests and support for edit_template mustache and cfengine
- Added test for edit_line, disabled because it's failing

Ticket: CFE-4591
Changelog: None

Co-authored-by: Qwen Code <qwen-code@alibaba.com>
Co-authored-by: Gemini <gemini-copilot@google.com>
Co-authored-by: Lars Erik Wik <lars.erik.wik@northern.tech>
Signed-off-by: Lars Erik Wik <lars.erik.wik@northern.tech>
@larsewi larsewi merged commit 63b8fea into cfengine:master Oct 27, 2025
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

4 participants