Comment 18 for bug 1748157

Revision history for this message
Yehezkel Bernat (yehezkelshb) wrote :

Thanks for the review!

I'd like to get a bit more details about some of the comments to understand what steps I can take to improve it. Please let me know if you prefer to take this discussion to another medium.

> - udev rules -- appear to be configured for works-by-default behaviour,
> some examples on how to configure for authorization-required would be
> nice

I'm not sure I understand what examples and what configuration this comment refers to.

> - No tests, a bit unfortunate

This is about the package or the upstream project? Upstream we have some tests (using umockdev). See here for CI:
https://travis-ci.org/intel/thunderbolt-software-user-space

> - File IO done via RAII-C++ classes, not exactly obvious when it happens

Any specific question that I can answer (or maybe even document in the code)?

> - No PolicyKit

My assumption (and is mentioned here and there in the code) is that eventually the tool will be better if it does start using PolicyKit for the privileged actions. It'd be nice to here what do you think about it or if there are guidelines from the distro on where, when and how to use it.

> It uses std::random_device for security uses -- I believe this is safe but
> direct use of getrandom(2) would not have questions about underlying C++
> library implementation choices.

Noted. flags=0 is good enough, yes?
(I will may have to make sure first that all the relevant distros include this syscall already.)

Thanks again!