| This is an automatically generated e-mail. To reply, visit: http://reviewboard-openlmi.rhcloud.com/r/1986/ |
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 allocating25*BUFLENbytes 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_getenforcein LMI_SELinuxServiceProvider.c / parse_port_instance_id()
* don't forget to freepartsin LMI_SELinuxServiceProvider.c / parse_port_range()
* mixing tabs with spacesin LMI_SELinuxServiceProvider.c / SetPortLabel()
* place thelmi_job_set_method_namebeforejobmgr_run_job
| src/selinux/selinux.c (Diff revision 1) | |||
|---|---|---|---|
| 33 | LMI_SELinuxService_Set_SystemName(service, lmi_get_system_name()); |
||
use
lmi_get_system_name_safe()
| src/selinux/selinux.c (Diff revision 1) | |||
|---|---|---|---|
| 60 | const gint CONTENT_SIZE = 25 * BUFLEN; |
||
use heap
| src/selinux/selinux.c (Diff revision 1) | |||
|---|---|---|---|
| 83 | while((read = getline (&line, &line_len, f)) != -1) { |
||
| 84 | if (g_regex_match(re, line, 0, &mi)) { |
||
| 85 | read = snprintf(line, BUFLEN - 1, "SELINUX=%s\n", (newstate == 0) ? "disabled" : |
||
| 86 | (newstate == 1) ? "permissive" : |
||
| 87 | (newstate == 2) ? "enforcing" : |
||
| 88 | "unknown"); |
||
| 89 | lmi_debug("SELINUX default state changed to %s", line); |
||
| 90 | } |
||
| 91 | g_strlcat(content, line, CONTENT_SIZE); |
||
| 92 | content_len += read; |
||
What if
SELINUX=*line is commented out?
| src/selinux/selinux.c (Diff revision 1) | |||
|---|---|---|---|
| 109 | lmi_debug("<== set_setlinux_default_state"); |
||
's/set_setlinux_default_state/set_selinux_default_state/'
| src/selinux/selinux.c (Diff revision 1) | |||
|---|---|---|---|
| 478 | if (include_input) |
||
| 479 | /* not interested in input parameters */ |
||
| 480 | return st; |
||
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 (Diff revision 1) | |||
|---|---|---|---|
| 75 | Build a dictionary o SELinux booleans from semanage output. |
||
's/\<o>/of/
| src/selinux/test/test_selinux.py (Diff revision 1) | |||
|---|---|---|---|
| 189 | │ └── d1-d2 -> /tmp/d1/d1-d2/ |
||
| 190 | ├── d4 |
||
| 191 | │ └── cycle -> /tmp/d4/ |
||
| 192 | ├── fsymlink -> /tmp/f.txt |
||
isn't
/tmpsupposed to be aTESTDIR?
| src/selinux/test/test_selinux.py (Diff revision 1) | |||
|---|---|---|---|
| 256 | # the order in which the setsebool commands are is called matters |
||
s/are is/are/
| src/selinux/test/test_selinux.py (Diff revision 1) | |||
|---|---|---|---|
| 312 | # security_genenforce() / security_getenforcemode() return -1 (disabled), 0 |
||
s/security_genenforce/security_getenforce/
| src/selinux/test/test_selinux.py (Diff revision 1) | |||
|---|---|---|---|
| 353 | self.assertEquals(res[0], 0) |
||
| 354 | inst = self._get_instance() |
||
| 355 | self._verify_selinux_states(inst) |
||
I'd also check that desired states were set.
- Michal Minar
On srpen 29th, 2014, 10:56 dop. UTC, Jan Synacek wrote:
|
Review request for OpenLMI Developers.
By Jan Synacek.
Updated Srp. 29, 2014, 10:56 dop.
Repository:
openlmi-providers
Description
Diffs
|