GNOME Bugzilla – Bug 309224
g_key_file_save_to_file missing
Last modified: 2013-10-04 16:18:39 UTC
i miss a posibility to save the content of a gkeyfile to a file example: g_key_file_save_to_file (GKeyFile *key_file, const gchar *file, GError **error)
saving to a file might involve locking issues, and currently GLib doesn't offer a way to lock a file portably (see bug #307131). a basic implementation - locking concerns notwithstanding - might look like this: /** * g_key_file_to_file: * @keyfile: a #GKeyFile * @file: the path of a filename to load, in the GLib file name encoding * @error: return location for a #GError, or %NULL * * This function save @keyfile contents inside @file. * Data is saved using g_file_set_contents(). * * Return value: %TRUE if the file was successfully written. */ gboolean g_key_file_to_file (GKeyFile *keyfile, const gchar *file, GError **error) { GError *write_error; gchar *data; gsize length; gboolean res; g_return_val_if_fail (keyfile != NULL, FALSE); g_return_val_if_fail (file != NULL, FALSE); write_error = NULL; data = g_key_file_to_data (keyfile, &length, &write_error); if (write_error) { g_propagate_error (error, write_error); return FALSE; } res = g_file_set_contents (file, data, length, &write_error); if (write_error) { g_propagate_error (error, write_error); g_free (data); return FALSE; } g_free (data); return res; }
*** Bug 527979 has been marked as a duplicate of this bug. ***
*** Bug 566231 has been marked as a duplicate of this bug. ***
Created attachment 156190 [details] [review] Implement g_key_file_to_file This is code I actually wrote for a project some time ago. I renamed it and put it into glib. Unlike the above suggestion, the error of g_key_file_to_data is ignored, just like the documentation recommends. I used fputs since configuration files can be symbolic links which would be destroyed if g_file_set_contents were used.
Review of attachment 156190 [details] [review]: ::: glib/gkeyfile.c @@ +1102,3 @@ + if (error) + *error = g_error_new (G_FILE_ERROR, G_FILE_ERROR_ACCES, + _("Writing failed.")); wouldn't it be better to use g_strerror(errno) instead of "Writing failed": At least change the text as opening the file for writing failed :) @@ +1105,3 @@ + return FALSE; + } + fputs (data, fp); would this cause an extra newline? If so fwrite() might be better. Also this should imho check the return value and set the error (and return FALSE). @@ +1106,3 @@ + } + fputs (data, fp); + fclose (fp); fclose() can fail as well.
Created attachment 156284 [details] [review] Implement g_key_file_to_file #2 Updated to always use g_file_error_from_errno and g_strerr, and to check all possible error conditions.
Review of attachment 156284 [details] [review]: This looks good now. Thanks for new version. ::: glib/gkeyfile.c @@ +1099,3 @@ + + if (!(fp = fopen (file, "w"))) + { should this be "wt" for text?
I have never heard of the "t" modifier. From what I see it could mean "transient" in some extended C library. Or did you mean something else? As far as I realize "w" alone is the traditional text mode, while the historical "b" used to mean "binary" and presumably has no effect on any recent C library.
Why not simply use g_file_set_contents() as comment 1 suggests? + if (error) + *error = g_error_new (G_FILE_ERROR, g_file_error_from_errno (errno), + g_strerror (errno)); Should use g_set_error_literal instead.
(In reply to comment #9) > Why not simply use g_file_set_contents() as comment 1 suggests? > I used fputs since configuration files can be symbolic links which would be destroyed if > g_file_set_contents were used.
(In reply to comment #8) > I have never heard of the "t" modifier. From what I see it could mean > "transient" in some extended C library. Or did you mean something else? As far > as I realize "w" alone is the traditional text mode, while the historical "b" > used to mean "binary" and presumably has no effect on any recent C library. First hit on google: http://www.cplusplus.com/reference/clibrary/cstdio/fopen/ but it sems to not matter on linux (have not even found this in the man page).
(In reply to comment #11) > First hit on google: > http://www.cplusplus.com/reference/clibrary/cstdio/fopen/ > but it sems to not matter on linux (have not even found this in the man page). I'm not seeing any mentioning of "wt" there. Just "w" and "b" in the way I said.
Created attachment 256402 [details] [review] Add g_key_file_write() To write a keyfile to disk.
bikeshedding over the name is more than welcome -- i probably didn't pick a good one on account of the fact that i don't really care :)
Comment on attachment 256402 [details] [review] Add g_key_file_write() >+ * g_key_file_write: given that it's the inverse of g_key_file_load_from_file(), I'd call it g_key_file_save_to_file() >+ contents = g_key_file_to_data (key_file, NULL, NULL); >+ success = g_file_set_contents (filename, contents, -1, error); seems silly to not do: gsize length; contents = g_key_file_to_data (key_file, &length, NULL); success = g_file_set_contents (filename, contents, length, error);
Created attachment 256485 [details] [review] Add g_key_file_save_to_file() To write a keyfile to disk.
Attachment 256485 [details] pushed as 5d7a7df - Add g_key_file_save_to_file()