Plasma Firewall

[Nate Graham]

I've got firewalld installed, yet I see an error asking me to install it 
upon opening the KCM: https://i.imgur.com/qQzYLwH.jpg

Nate


On 9/17/20 5:20 AM, Tomaz Canabrava wrote:
> People, I have spend a great deal of time fine tuning the Firewall, I 
> hope it's ready for a release.
> 
> On Wed, Jun 3, 2020 at 6:33 PM Tomaz Canabrava <tcanabrava at kde.org 
> <mailto:tcanabrava at kde.org>> wrote:
> 
>     Lots of nitpicks fixed, still missing some, I'll finish the rest today.
> 
> 
>     On Fri, May 22, 2020 at 6:10 PM Ivan Čukić <ivan.cukic at kde.org
>     <mailto:ivan.cukic at kde.org>> wrote:
> 
>         Hi Kai,
> 
> 
>         This is something that Plasma has been missing for a long time.
>         Thanks to
>         everyone involved!
> 
> 
>         A few comments from a quick run-through over kcm/core:
> 
>         # Style issues
> 
>         - Some enums use MACRO_STYLE, some NormalStyle
> 
>         - Some one-line if bodies in the same file have {}, some do not
> 
>         - Double plural interfaces_names
>         https://invent.kde.org/tcanabrava/plasma-firewall/-/blob/master/kcm/core/firewallclient.cpp#L56
> 
>         - Personal preference - error handling in the if instead of out.
>         This:
>            if (!m_currentBackend) { return nullptr; }
>            return proper;
>         instead of this:
>            if (m_currentBackend) { return proper; }
>            return nullptr;
> 
>         - Missing m_ prefix for member variables:
>         https://invent.kde.org/tcanabrava/plasma-firewall/-/blob/master/kcm/core/
>         profile.h#L108
>         <https://invent.kde.org/tcanabrava/plasma-firewall/-/blob/master/kcm/core/profile.h#L108>
> 
>         - s/getRule/ruleAt/ - we don't usually have 'get' in the API if
>         we can avoid
>         it:
>         https://invent.kde.org/tcanabrava/plasma-firewall/-/blob/master/kcm/core/
>         ifirewallclientbackend.h#L50
>         <https://invent.kde.org/tcanabrava/plasma-firewall/-/blob/master/kcm/core/ifirewallclientbackend.h#L50>
> 
>         - Could use a pipe through clang-format
> 
> 
>         # Non-style issues:
> 
>         - Should be ||, not && in:
>         https://invent.kde.org/tcanabrava/plasma-firewall/-/blob/master/kcm/core/
>         rulelistmodel.cpp#L37
>         <https://invent.kde.org/tcanabrava/plasma-firewall/-/blob/master/kcm/core/rulelistmodel.cpp#L37>
> 
>         - Can be a const or a ref-to-const, no need for qAsConst after:
>         https://invent.kde.org/tcanabrava/plasma-firewall/-/blob/master/kcm/core/
>         firewallclient.cpp#L58
>         <https://invent.kde.org/tcanabrava/plasma-firewall/-/blob/master/kcm/core/firewallclient.cpp#L58>
> 
>         - Should add m_currentBackend = nullptr; after delete
>         https://invent.kde.org/tcanabrava/plasma-firewall/-/blob/master/kcm/core/
>         firewallclient.cpp#L248
>         <https://invent.kde.org/tcanabrava/plasma-firewall/-/blob/master/kcm/core/firewallclient.cpp#L248>
>         (otherwise you get UB on 270 if no plugins have been found)
> 
>         - Logic-wise - it would be better to redesign (if possible) not
>         to allow
>         m_currentBackend == nullptr instead of asserting and checking in
>         each function
>         (for example, to show a UI that can not call anything in the
>         backend is no
>         backend is loaded)
> 
>         - Wrong order - sort m_profiles and then replace it with a new
>         contents?
>         https://invent.kde.org/tcanabrava/plasma-firewall/-/blob/master/kcm/core/
>         ifirewallclientbackend.cpp#L36
>         <https://invent.kde.org/tcanabrava/plasma-firewall/-/blob/master/kcm/core/ifirewallclientbackend.cpp#L36>
> 
>         - cbegin/cend instead of begin/end:
>         https://invent.kde.org/tcanabrava/plasma-firewall/-/blob/master/kcm/core/
>         ifirewallclientbackend.cpp#L42
>         <https://invent.kde.org/tcanabrava/plasma-firewall/-/blob/master/kcm/core/ifirewallclientbackend.cpp#L42>
> 
> 
>         - [&name] instead of [name]
>         https://invent.kde.org/tcanabrava/plasma-firewall/-/blob/master/kcm/core/
>         ifirewallclientbackend.cpp#L43
>         <https://invent.kde.org/tcanabrava/plasma-firewall/-/blob/master/kcm/core/ifirewallclientbackend.cpp#L43>
> 
>         - mark member functions that can be const as const
>         https://invent.kde.org/tcanabrava/plasma-firewall/-/blob/master/kcm/core/
>         ifirewallclientbackend.h#L89
>         <https://invent.kde.org/tcanabrava/plasma-firewall/-/blob/master/kcm/core/ifirewallclientbackend.h#L89>
> 
>         - else if for chained checks of reader.name
>         <http://reader.name>() == something
>         https://invent.kde.org/tcanabrava/plasma-firewall/-/blob/master/kcm/core/
>         profile.cpp#L163
>         <https://invent.kde.org/tcanabrava/plasma-firewall/-/blob/master/kcm/core/profile.cpp#L163>
> 
>         - !...isEmpty is not needed - adds.contains(":") covers !...isEmpty
>         https://invent.kde.org/tcanabrava/plasma-firewall/-/blob/master/kcm/core/
>         rule.cpp#L46
>         <https://invent.kde.org/tcanabrava/plasma-firewall/-/blob/master/kcm/core/rule.cpp#L46>
> 
>         - Assigning long-lived parents is naughty:
>         https://invent.kde.org/tcanabrava/
>         plasma-firewall/-/blob/master/kcm/backends/ufw/ufwclient.cpp#L318 <https://invent.kde.org/tcanabrava/plasma-firewall/-/blob/master/kcm/backends/ufw/ufwclient.cpp#L318>
> 
> 
> 
>         So much from me at this point.
> 
>         Cheers,
>         Ivan
> 
> 
> 
>         -- 
>         dr Ivan Čukić
>         ivan at cukic.co <mailto:ivan at cukic.co>, https://cukic.co/
>         gpg key fingerprint: 8FE4 D32F 7061 EA9C 8232  07AE 01C6 CE2B
>         FF04 1C12
> 
>