-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard-openlmi.rhcloud.com/r/1986/#review3277
-----------------------------------------------------------
Second part of my review.
### Additional notes:
in LMI_SELinuxPortProvider.c / LMI_SELinuxPortEnumInstances()
* `s/broken/broker/`
* `s/to free to/to free/`
* `ranges` (`GSList`) are not freed (just ports they refer to)
in LMI_SELinuxServiceProvider.c / parse_selinux_sysconf()
* I'd avoid allocating `25*BUFLEN` bytes on a stack. Please use heap. Although
it isn't totally safe, I'd also recommend to get the file size and use it as a
buffer size (+1).
in LMI_SELinuxServiceProvider.c / LMI_SELinuxServiceEnumInstances()
* s/security_genenforce/security_getenforce
in LMI_SELinuxServiceProvider.c / parse_port_instance_id()
* don't forget to free `parts`
in LMI_SELinuxServiceProvider.c / parse_port_range()
* mixing tabs with spaces
in LMI_SELinuxServiceProvider.c / SetPortLabel()
* place the `lmi_job_set_method_name` before `jobmgr_run_job`
src/selinux/selinux.c
<
http://reviewboard-openlmi.rhcloud.com/r/1986/#comment1822>
use `lmi_get_system_name_safe()`
src/selinux/selinux.c
<
http://reviewboard-openlmi.rhcloud.com/r/1986/#comment1823>
use heap
src/selinux/selinux.c
<
http://reviewboard-openlmi.rhcloud.com/r/1986/#comment1825>
What if `SELINUX=*` line is commented out?
src/selinux/selinux.c
<
http://reviewboard-openlmi.rhcloud.com/r/1986/#comment1824>
's/set_setlinux_default_state/set_selinux_default_state/'
src/selinux/selinux.c
<
http://reviewboard-openlmi.rhcloud.com/r/1986/#comment1826>
If believe
if (!include_output)
return st;
is what you really want. But input arguments could be filled as well - they are
already defined in `__MethodParameters_*` classes.
src/selinux/test/test_selinux.py
<
http://reviewboard-openlmi.rhcloud.com/r/1986/#comment1827>
's/\<o>/of/
src/selinux/test/test_selinux.py
<
http://reviewboard-openlmi.rhcloud.com/r/1986/#comment1828>
isn't `/tmp` supposed to be a `TESTDIR`?
src/selinux/test/test_selinux.py
<
http://reviewboard-openlmi.rhcloud.com/r/1986/#comment1829>
`s/are is/are/`
src/selinux/test/test_selinux.py
<
http://reviewboard-openlmi.rhcloud.com/r/1986/#comment1830>
`s/security_genenforce/security_getenforce/`
src/selinux/test/test_selinux.py
<
http://reviewboard-openlmi.rhcloud.com/r/1986/#comment1831>
I'd also check that desired states were set.
- Michal Minar
On Srp. 29, 2014, 10:56 dop., Jan Synacek wrote:
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard-openlmi.rhcloud.com/r/1986/
-----------------------------------------------------------
(Updated Srp. 29, 2014, 10:56 dop.)
Review request for OpenLMI Developers.
Repository: openlmi-providers
Description
-------
implement SELinux provider
In case diffs don't work, I'll track changes in my WIP branch until the patch is
merged:
https://git.fedorahosted.org/cgit/openlmi-providers.git/log/?h=selinux-devel
Diffs
-----
CMakeLists.txt 463da8c63a314dd34595710a127a0429f89edc49
mof/60_LMI_SELinux.mof PRE-CREATION
mof/60_LMI_SELinux.reg PRE-CREATION
mof/60_LMI_SELinux_MethodParameters.mof PRE-CREATION
mof/CMakeLists.txt 5e7aaa7f08d7f57277a0215970ba49064d6e0e69
src/.dir-locals.el PRE-CREATION
src/CMakeLists.txt 58d72f596a1675e16ff06f437333859b54c00800
src/libs/jobmanager/lmi_job.c 004e6a72c9f5c36acfa3f4062a5a036ab0843d39
src/libs/libopenlmi/openlmi.h 8f4770cde0d826fa95dabaa5a700934030f7bc90
src/libs/libopenlmi/openlmi.c da660a5bdb5c9fc37aa17031700bf3d93596d43a
src/logicalfile/CMakeLists.txt 0448f1e8c04dba47b767edd4b84f049fe5e0cf26
src/logicalfile/file.h 4caf84dd4db8dafc705fdd99738ff0e886db6323
src/selinux/90_LMI_SELinux_Profile.mof.skel PRE-CREATION
src/selinux/CMakeLists.txt PRE-CREATION
src/selinux/LMI_AffectedSELinuxJobElementProvider.c PRE-CREATION
src/selinux/LMI_AssociatedSELinuxJobMethodResultProvider.c PRE-CREATION
src/selinux/LMI_HostedSELinuxServiceProvider.c PRE-CREATION
src/selinux/LMI_SELinuxBooleanProvider.c PRE-CREATION
src/selinux/LMI_SELinuxInstModificationProvider.c PRE-CREATION
src/selinux/LMI_SELinuxJobProvider.c PRE-CREATION
src/selinux/LMI_SELinuxMethodResultProvider.c PRE-CREATION
src/selinux/LMI_SELinuxPortProvider.c PRE-CREATION
src/selinux/LMI_SELinuxServiceHasElementProvider.c PRE-CREATION
src/selinux/LMI_SELinuxServiceProvider.c PRE-CREATION
src/selinux/cmpiLMI_SELinux-cimprovagt PRE-CREATION
src/selinux/selinux.h PRE-CREATION
src/selinux/selinux.c PRE-CREATION
src/selinux/test/README PRE-CREATION
src/selinux/test/__init__.py PRE-CREATION
src/selinux/test/test_selinux.py PRE-CREATION
Diff:
http://reviewboard-openlmi.rhcloud.com/r/1986/diff/
Testing
-------
Thanks,
Jan Synacek