Hi Frédéric,
On Wed, Oct 03, 2012, Frédéric Danis wrote:
> Add functions to read and write to .conf files.
> Convert adapter "config" file to "adapter.conf".
> ---
> .gitignore | 1 +
> Makefile.am | 3 +-
> Makefile.tools | 6 +-
> src/keyfile.c | 282 +++++++++++++++++++++++++++++++++++++++++++++++++++
> src/keyfile.h | 51 ++++++++++
> src/storage.c | 100 +++++++-----------
> test/test-keyfile.c | 252 +++++++++++++++++++++++++++++++++++++++++++++
> 7 files changed, 627 insertions(+), 68 deletions(-)
> create mode 100644 src/keyfile.c
> create mode 100644 src/keyfile.h
> create mode 100644 test/test-keyfile.c
The keyfile.[ch] files should be split into their own patch along with
test-keyfile.c.
> +int keyfile_put_string(const char *pathname, const char *group,
> + const char *key, const char *value)
> +{
> + GKeyFile *key_file;
> + char *data;
> + FILE *file;
> +
> + key_file = g_key_file_new();
> +
> + g_key_file_load_from_file(key_file, pathname, 0, NULL);
> +
> + g_key_file_set_string(key_file, group, key, value);
> +
> + data = g_key_file_to_data(key_file, NULL, NULL);
> + file = fopen(pathname, "w");
> + fputs(data, file);
> + fclose(file);
It seems like there's some error checking missing here as both
g_key_file_load_from_file and fopen could fail.
> +}
> +
> +
> +int keyfile_get_boolean(const char *pathname, const char *group,
Two consecutive empty lines above. Please remove one.
> + const char *key, gboolean *value)
> +{
> + GKeyFile *key_file;
> + GError *error = NULL;
Please don't reuse the symbol name "error" since we already have that
for error logging. Use gerr instead
> + int err = 0;
This initialization upon declaration is unnecessary. Set to 0 at the end
of the success path instead.
> + if (error) {
> + g_error_free(error);
> + err = -EINVAL;
> + }
< i.e. here >
> +
> +failed:
> + g_key_file_free(key_file);
> + errno = -err;
Don't mix errno into this. It should be considered "owned" by libc and
we should only read it and not write to it. Return the error in the
function return value instead.
> + return keyfile_put_integer(filename, "General", "DiscovTO", timeout);
I really dislike this "TO" short-hand for timeout. I think we can use
longer and more readable names in the INI-format files. In this case
DiscoverableTimeout should be perfectly fine.
> + return keyfile_put_integer(filename, "General", "PairTO", timeout);
Same here. PairableTimeout should be fine.
Johan
--
To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
