[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