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!

-- 
You received this bug notification because you are a member of Ubuntu
Bugs, which is subscribed to Ubuntu.
https://bugs.launchpad.net/bugs/1748157

Title:
   [MIR] thunderbolt-tools

To manage notifications about this bug go to:
https://bugs.launchpad.net/ubuntu/+source/thunderbolt-tools/+bug/1748157/+subscriptions

-- 
ubuntu-bugs mailing list
ubuntu-bugs@lists.ubuntu.com
https://lists.ubuntu.com/mailman/listinfo/ubuntu-bugs

Reply via email to