| This is an automatically generated e-mail. To reply, visit: http://reviewboard-openlmi.rhcloud.com/r/430/ |
| src/locale/LMI_LocaleProvider.c (Diff revision 4) | |||
|---|---|---|---|
| 41 | tmps = (char *) calloc((strlen(NAM ## EnvVar) + strlen((NAM)->chars) + 1), sizeof(char)); \ |
||
Couldn't this solve simple g_strdup_printf() call?
| src/locale/localed.c (Diff revision 4) | |||
|---|---|---|---|
| 50 | GError *error = NULL; |
||
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 (Diff revision 4) | |||
|---|---|---|---|
| 65 | result = g_dbus_proxy_call_sync(proxy, "Get", g_variant_new("(ss)", LOCALE_INTERFACE, "Locale"), |
||
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 (Diff revision 4) | |||
|---|---|---|---|
| 70 | while (g_variant_iter_loop(arr, "&s", &env_str)) { |
||
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#gvariant-format-strings-pointers
| src/locale/localed.c (Diff revision 4) | |||
|---|---|---|---|
| 136 | if (error) g_error_free(error); |
||
Tip: Can use g_clear_error() without the check instead. Similar to g_clear_object().
| src/locale/localed.c (Diff revision 4) | |||
|---|---|---|---|
| 151 | if (cloc->VConsoleKeymap) free(cloc->VConsoleKeymap); |
||
Tip: or use g_free() that does NULL check for you.
Minor issues, overall it looks good.
- Tomáš Bžatek
On květen 29th, 2014, 7:12 dop. UTC, Vitezslav Crhonek wrote:
|
Review request for OpenLMI Developers.
By Vitezslav Crhonek.
Updated Kvě. 29, 2014, 7:12 dop.
Repository:
openlmi-providers
Description
Diffs
|