-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard-openlmi.rhcloud.com/r/430/#review2684
-----------------------------------------------------------
src/locale/LMI_LocaleProvider.c
<
http://reviewboard-openlmi.rhcloud.com/r/430/#comment1487>
Couldn't this solve simple g_strdup_printf() call?
src/locale/localed.c
<
http://reviewboard-openlmi.rhcloud.com/r/430/#comment1488>
You're not using error anywhere, just setting and it freeing afterwards. You
should pass NULL instead and check for the return value (result == NULL or FALSE
automatically means error and the passed GError variable is set).
src/locale/localed.c
<
http://reviewboard-openlmi.rhcloud.com/r/430/#comment1490>
Doesn't g_dbus_proxy_get_cached_property() on the proxy constructed with
org.freedesktop.locale1 interface do the trick easier? I don't remember whether it
works with Lennart's interfaces...
src/locale/localed.c
<
http://reviewboard-openlmi.rhcloud.com/r/430/#comment1491>
If you remove the ampersand operator, you don't need to call strdup() in the next
step. Don't forget that you own the memory now and should take care of proper cleanup
(not this case though).
See
https://developer.gnome.org/glib/unstable/gvariant-format-strings.html#gv...
src/locale/localed.c
<
http://reviewboard-openlmi.rhcloud.com/r/430/#comment1489>
Tip: Can use g_clear_error() without the check instead. Similar to g_clear_object().
src/locale/localed.c
<
http://reviewboard-openlmi.rhcloud.com/r/430/#comment1492>
Tip: or use g_free() that does NULL check for you.
Minor issues, overall it looks good.
- Tomáš Bžatek
On Kvě. 29, 2014, 7:12 dop., Vitezslav Crhonek wrote:
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard-openlmi.rhcloud.com/r/430/
-----------------------------------------------------------
(Updated Kvě. 29, 2014, 7:12 dop.)
Review request for OpenLMI Developers.
Repository: openlmi-providers
Description
-------
New provider: Locale
Diffs
-----
CMakeLists.txt bcea09707d1d87db374cf6561d00a19258b54b3b
README 9e7a7abce940c3dea55e0d2a6cf6daed315e8361
mof/60_LMI_Locale.mof PRE-CREATION
mof/CMakeLists.txt cbc971b1baa3b9400c644f7abdfc0c272bbabf65
openlmi-providers.spec dc975d5f0fd169d49227b3749e0fa3f96e5184f9
src/CMakeLists.txt e0fbb4e05236fc10164f359b5a13f6406aacbda1
src/locale/90_LMI_Locale_Profile.mof.skel PRE-CREATION
src/locale/CMakeLists.txt PRE-CREATION
src/locale/LMI_LocaleProvider.c PRE-CREATION
src/locale/cmpiLMI_Locale-cimprovagt PRE-CREATION
src/locale/localed.h PRE-CREATION
src/locale/localed.c PRE-CREATION
Diff:
http://reviewboard-openlmi.rhcloud.com/r/430/diff/
Testing
-------
Thanks,
Vitezslav Crhonek