[PATCH 0/3] System config file accessor cleanup

Linus Torvalds torvalds at linux-foundation.org
Thu Jan 10 20:01:29 PST 2013


Ok, so this patch-series started out as an attempty to modularize the 
configuration reading/writing a bit, so that we could at least look at 
reading from different sources.

However, it ended up being just a cleanup series. There should be very 
little actual semantic differences, but a fair amount of lines changed.

The first patch is a pure mindless cleanup: it renames the oddly named 
'input_units' and 'output_units' variables (the naming is purely 
historical: the whole project started out mostly as an XML engine, and 
'input_units' is the XML parsing input side, while 'output_units' is then 
the user-facing units).

So that first patch renames 'input_units' to be 'xml_parsing_units', and 
'output_units' is just the 'units' part of the preferences. It also moves 
the details to a new header (prefs.h), because none of this is really 
about gtk in any way, so having it in 'display-gtk.h' doesn't make much 
sense.

The second patch is the smallest of the three, and just introduces the 
concept of the 'default preferences', which are initialized from the 
system settings. It uses the "correct" POSIX way (the LC_MEASUREMENT 
environment variable, falling back on LC_ALL and LANG as required), and 
that works on Linux, but I have no idea if OSX/Windows have some other way 
to specify the locale for measurements.

Anyway, apart from making Imperial units the default in the US (in the 
absense of any explicit settings) it makes no other real changes. And if 
somebody from Burma or Liberia wants to fix the locale for that, go right 
ahead, I couldn't find it in myself to care.

The third patch is the largest, because it then actually changes how we 
read and write the configuration information. Having that default state, 
it now only writes config info if it is different from the system 
defaults. Similarly, when reading, missing values no longer mean "false", 
they mean "use the system default".

This required changing the interface for PREF_BOOL reading, which used to 
be able to only return (void *)0 and (void *)1 for false and true 
respectively. And while at it, I changed the interface completely, so that 
it's much easier to use.

I've tested the "linux.c" changes, and I *tried* to make the macos.c and 
windows.c changes look correct, but I couldn't test them. So people with 
other platforms should really check what I did. But the code is both 
simpler and cleaner, and can now return -1 for "I have no config entry" 
for the boolean case.

Anyway, it should have no actual semantic changes, modulo bugs. Except now 
we can change the defaults (like which columns are visible etc by 
default), and if users haven't changed the defaults, they get the new 
ones. And you can actually see in the config data whether some default 
entry has been changed, because only non-default values are written out 
(well, right now I do that only for the booleans, I haven't done the 
"only save if non-default" logic for floating point and strings).

The booleans are now all saved/restored with some macro patterns, which 
makes the code smaller and more readable. And much more amenable to 
changing in the future. So instead of a line like

    subsurface_set_conf("feet", PREF_BOOL, BOOL_TO_PTR(prefs.units.length == FEET));

we now have

    SAVE_UNIT("feet", length, FEET);

which should make it easier to manage. Especially if we then start having 
flags like "save to xml" vs "save to system config" vs "save to our own 
config file".

                   Linus


More information about the subsurface mailing list