This is an automatically generated e-mail. To reply, visit: http://reviewboard-openlmi.rhcloud.com/r/1986/

First part of my review - I'll continue later.

mof/60_LMI_SELinux.mof (Diff revision 1)
		21	[ Version("0.0.1"),

Bump the version to the next OpenLMI bundle release.

Applies to all the classes.

mof/60_LMI_SELinux.mof (Diff revision 1)
		25	[ Key, Override("InstanceID") ]

Please, add Implemented(True) qualifiers to implemented properties.

mof/60_LMI_SELinux.mof (Diff revision 1)
		30	Description("Class representing an SELinux policy boolean.") ]

s/\<an>/a/

mof/60_LMI_SELinux.mof (Diff revision 1)
		42	"Class representing an SELinux port. It can encompass multiple "

s/\<an>/a/

mof/60_LMI_SELinux.mof (Diff revision 1)
		113	[ IN, Description("An SELinux port to change.") ]

s/\<An>/A/

mof/60_LMI_SELinux.mof (Diff revision 1)
		126	"Set label on an SELinux file."),

's/\<an>/a/'
Ending commenting of this flaw.

mof/60_LMI_SELinux.mof (Diff revision 1)
		172	[ IN, OUT, Description("SELinux file(s) to change. On output, all files that have unexpected SELinux context") ]

I don't see a way how a single reference to a unix file could refer to multiple files. If it's really possible, it should be mentioned in doc string.

mof/60_LMI_SELinux.mof (Diff revision 1)
		191	Description("Association class the connects the SELinux system service with its elements."),

's/Association class the connects/Associates/'

mof/60_LMI_SELinux.mof (Diff revision 1)
		204	class LMI_HostedSELinuxService : CIM_HostedService

I would override at least Dependent and make it refer to LMI_SELinuxService.

mof/60_LMI_SELinux.mof (Diff revision 1)
		240	class LMI_SELinuxInstModification : CIM_InstModification {

LMI_SELinuxInstCreation and LMI_SELinuxInstDeletion are missing. They'll need an analogue of LMI_SELinuxInstModiciationProvider.c.

mof/60_LMI_SELinux.reg (Diff revision 1)
		1	[LMI_SELinuxElement]

I believe this file should be generated during build.

src/libs/jobmanager/lmi_job.c (Diff revision 1)
void lmi_job_set_data(LmiJob *job,
1679	gboolean lmi_job_set_state(LmiJob *job,	1679	gboolean lmi_job_set_state(LmiJob *job,

Please, make this function static. Thanks :)

src/libs/jobmanager/lmi_job.c (Diff revision 1)
gboolean lmi_job_finish_ok_with_code(LmiJob *job, guint32 exit_code)
	1863	gchar verror[BUFSIZ + 1];
	1864	va_list args;
	1865
	1866	va_start(args, error);
	1867	vsnprintf(verror, BUFSIZ, error, args);
	1868	va_end(args);
	1869

Good catch!

src/libs/libopenlmi/openlmi.c (Diff revision 1)
		787	gchar lmi_strip_str(gchar str, gchar c)

strip suggests removing of something from the beginning or the end. Perhaps lmi_ditch_char could be more appropriate.

src/selinux/LMI_AffectedSELinuxJobElementProvider.c (Diff revision 1)
		154	return KDefaultAssociatorNames(

IMHO Target of RestoreLabels(), SetPortLabel(), SetFileLabel(), SetBoolean are affected elements of SELinuxJob.

src/selinux/LMI_SELinuxInstModificationProvider.c (Diff revision 1)
		31	selinux_provider_init(LMI_SELinuxInstModification_ClassName, FALSE, provider_config_defaults, _cb, ctx);

's/FALSE/TRUE/'

src/selinux/LMI_SELinuxJobProvider.c (Diff revision 1)
		74	continue; /* TODO this won't work, will cycle forever */

You've said it yourself :).

I'll fix it in software provider. Thanks for noticing.

src/selinux/LMI_SELinuxJobProvider.c (Diff revision 1)
		78	continue; /* TODO this won't work, will cycle forever */

And once again.

- 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

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)

View Diff