After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 309224 - g_key_file_save_to_file missing
g_key_file_save_to_file missing
Status: RESOLVED FIXED
Product: glib
Classification: Platform
Component: gkeyfile
unspecified
Other Linux
: Normal enhancement
: ---
Assigned To: gtkdev
gtkdev
: 527979 566231 (view as bug list)
Depends on:
Blocks:
 
 
Reported: 2005-06-30 19:12 UTC by Jochen Baier
Modified: 2013-10-04 16:18 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Implement g_key_file_to_file (2.26 KB, patch)
2010-03-15 16:18 UTC, Christian Dywan
needs-work Details | Review
Implement g_key_file_to_file #2 (2.61 KB, patch)
2010-03-16 16:34 UTC, Christian Dywan
none Details | Review
Add g_key_file_write() (2.81 KB, patch)
2013-10-03 16:40 UTC, Allison Karlitskaya (desrt)
none Details | Review
Add g_key_file_save_to_file() (2.88 KB, patch)
2013-10-04 16:16 UTC, Allison Karlitskaya (desrt)
committed Details | Review

Description Jochen Baier 2005-06-30 19:12:07 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)
Comment 1 Emmanuele Bassi (:ebassi) 2006-05-20 15:22:21 UTC
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;
}
Comment 2 Matthias Clasen 2008-10-11 19:17:52 UTC
*** Bug 527979 has been marked as a duplicate of this bug. ***
Comment 3 Matthias Clasen 2009-01-02 23:25:38 UTC
*** Bug 566231 has been marked as a duplicate of this bug. ***
Comment 4 Christian Dywan 2010-03-15 16:18:08 UTC
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.
Comment 5 Stefan Sauer (gstreamer, gtkdoc dev) 2010-03-15 16:48:50 UTC
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.
Comment 6 Christian Dywan 2010-03-16 16:34:12 UTC
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.
Comment 7 Stefan Sauer (gstreamer, gtkdoc dev) 2010-03-16 16:39:03 UTC
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?
Comment 8 Christian Dywan 2010-03-16 16:46:34 UTC
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.
Comment 9 Christian Persch 2010-03-16 19:03:30 UTC
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.
Comment 10 Christian Dywan 2010-03-16 19:52:33 UTC
(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.
Comment 11 Stefan Sauer (gstreamer, gtkdoc dev) 2010-03-16 20:30:43 UTC
(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).
Comment 12 Christian Dywan 2010-03-16 20:47:31 UTC
(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.
Comment 13 Allison Karlitskaya (desrt) 2013-10-03 16:40:17 UTC
Created attachment 256402 [details] [review]
Add g_key_file_write()

To write a keyfile to disk.
Comment 14 Allison Karlitskaya (desrt) 2013-10-03 16:57:29 UTC
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 15 Dan Winship 2013-10-03 21:02:36 UTC
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);
Comment 16 Allison Karlitskaya (desrt) 2013-10-04 16:16:43 UTC
Created attachment 256485 [details] [review]
Add g_key_file_save_to_file()

To write a keyfile to disk.
Comment 17 Allison Karlitskaya (desrt) 2013-10-04 16:18:35 UTC
Attachment 256485 [details] pushed as 5d7a7df - Add g_key_file_save_to_file()