[systemd-devel] [PATCH v9000 2/3] timedated: gather timezone from /etc/localtime sym target

[Frederic Crozat]

Le dimanche 12 août 2012 à 22:36 -0700, Shawn Landden a écrit :
> /etc/localtime -> /usr/share/zoneinfo/...
> 
> or
> 
> /etc/localtime -> ../usr/share/zoneinfo/...
> 
> (note, ../usr is not the same if /etc is a symlink, as this isn't
> using canonicalize_file_name())
> 
> keep other method for now, consider dropping later.
> 
> Supporting relative links here are problematic as timezones in
> /usr/share/zoneinfo are often themselves symlinks (and symlinks to
> symlinks), so this implamentation only supports absolute symlinks
> "/usr/share/zoneinfo/" and relative symlinks starting with
> "../usr/share/zoneinfo/"
> 
> From TODO (kay sievers):
> * kill /etc/timezone handling entirely? What does it provide?
>   - /etc/localtime carries the same information already:
>       $ ls -l /etc/localtime; cat /etc/timezone
>       lrwxrwxrwx 1 root root 33 Jul 27 09:55 /etc/localtime -> /usr/share/zoneinfo/Europe/Berlin
>       Europe/Berlin
>   - systemd enforces /usr to be available at bootup, so we can
>     enforce the use of the symlink
> ---
>  src/timedate/timedated.c |   50 ++++++++++++++++++++++++++++++++++++----------
>  1 file changed, 40 insertions(+), 10 deletions(-)
> 
> diff --git a/src/timedate/timedated.c b/src/timedate/timedated.c
> index 09fd808..c3067c8 100644
> --- a/src/timedate/timedated.c
> +++ b/src/timedate/timedated.c
> @@ -74,6 +74,9 @@
>          BUS_GENERIC_INTERFACES_LIST             \
>          "org.freedesktop.timedate1\0"
>  
> +/* Must start and end with '/' */
> +#define ZONEINFO_PATH "/usr/share/zoneinfo/"
> +
>  const char timedate_interface[] _introspect_("timedate1") = INTERFACE;
>  
>  typedef struct TZ {
> @@ -152,16 +155,14 @@ static void verify_timezone(void) {
>                  return;
>  
>          p = strappend("/usr/share/zoneinfo/", tz.zone);
                        ^^^^^^^^^^^^^^^^^^^^^
it would be better to replace this with the macro you added
(ZONEINFO_PATH)

> -        if (!p) {
> -                log_oom();
> -                return;
> -        }
> +        if (!p)
> +                return (void)log_oom();

I would keep the way the log_oom / return was used initially and not
change it.


-- 
Frederic Crozat <fcrozat at suse.com>
SUSE