GNOME Bugzilla – Bug 420686
g_key_file_to_data alters original data
Last modified: 2007-03-22 09:21:05 UTC
Please describe the problem: In gkeyfile.c, g_key_file_to_data() unconditionally appends a newline between groups (the comment is simply "separate groups by at least an empty line"). The resulting string is, therefore, different from the original contents of the file, by 1 newline character per group. If one writes back the contents of this string to the original file, that file then grows. Bug 344868 explains why this was done, but I do not agree with that bug, specifically the comment: "Keyfiles written by g_key_file_to_data are a bit hard to read since the groups aren't separated by empty lines." is only valid for keyfiles that have the lack of newlines anyway. In my view, the exact contents of the original file should be maintained when calling g_key_file_to_data. I've seen an example of this problem 'in the wild' (attachment 58956 [details] [review]) and another user has reported the same problem to the gtk-list. I have a patch to remove the extra newlines, and to test that g_key_file_to_data leaves that data unchanged. Steps to reproduce: Actual results: Expected results: Does this happen every time? Other information:
I filed bug 344868 because I _did_ find that "Keyfiles written by g_key_file_to_data are a bit hard to read since the groups aren't separated by empty lines." which does apply for keyfiles you generate entirely from code, i.e. g_key_file_new() + add keys + to_data. I don't think there's any guarantee implied that a load_from_file + a to_data will give you the exact same string as the original. In fact it will not; for example --snip-- [Group1] [Group2] --snip-- will be re-serialised with just one newline.
OK, Christian, I believe you and I understand your test case :) To be honest, I can't think of an ideal solution, short of code that inserts a single newline before a group, if there wasn't one already. This would ensure that the file only grows to a finite size and would maintain all 'nicely formatted' keyfiles as is. However, this is not the behaviour I currently see; to_data() *always* appends extra newlines, in addition to any that are in the original data. Hence the file grows indefinitely.
(In reply to comment #2) > However, this is not the behaviour I currently see; to_data() *always* appends > extra newlines, in addition to any that are in the original data. Hence the > file grows indefinitely. This behaviour was identified and fixed in bug 349825, using exactly the method you proposed. Which version of glib are you testing, as the unconditional growth was introduced in 2.11.4 and fixed in 2.12.2?
Looks like bug 349825 but that's supposed to be fixed.
Created attachment 85080 [details] [review] Add a test case for reloading GKeyFiles As this test case indicates the bug was not fixed.
Thanks for the testcase
For the record, I'm seeing this in 2.12.9, 2.12.11, and today's trunk. Chris's test case is very similar to the one I was working from before I even filed this bug, and at that time I believed the operation should be idempotent. Then Christian made the point about programatically generated files, so I'm confused as to exactly what the functionality should be. Should to_data() be idempotent or should it only (and always) guarantee a single blank line between groups?
Btw, my testing was without the G_KEY_FILE_KEEP_COMMENTS flags, so that's why I didn't see the problem.
Created attachment 85086 [details] [review] Make reloading GKeyFiles idempotent (after a single beautifying pass) This patch endeavours to insert only a single line between the key=value pairs of one group and the next [group]. In the process of doing this I changed the semantics of the comments slightly. Previously: #top comment1 #top comment2 [group1] #key1 comment key1=value #key2 comment key2=value #group2 comment [group2] #key3 comment Now the comment before the group is tightly bound to that group and we use blank lines to break up comment blocks, viz: #top comment1 #top comment2 <blank> #group1 comment #group1 comment [group1] #key1 comment key1=value #group2 comment group2
Created attachment 85087 [details] [review] Make reloading GKeyFiles idempotent (after a single beautifying pass) This time without changing comment semantics.
Looks fine, please commit to both branches.
Commited to trunk r5431 and glib-2.12 r5432: 2007-03-22 Chris Wilson <chris@chris-wilson.co.uk> * glib/gkeyfile.c: Track whether the last key=value pair in a group is a blank line and during to_data() only insert a new blank line betweens group in its absence. This allows the beautification of the GKeyFile and prevents newlines being inserted indefinitely. (#420686) * tests/keyfile-test.c (test_reload_idempotency): Test that after a single beautification pass, g_key_file_to_data() does not alter its input data. This will be defeated by someone using g_key_file_set_comment(keyfile, group, NULL, "\nComment\n") but only on the first pass, subsequent reloads should not add further newlines.