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

2012-08-14 Thread 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 fcro...@suse.com
SUSE

___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

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

2012-08-14 Thread Frederic Crozat 
Le mardi 14 août 2012 à 10:32 +0200, Frederic Crozat a écrit :
 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)

And I've found another occurrence of /usr/share/zoneinfo which would
need to be fixed.

-- 
Frederic Crozat fcro...@suse.com
SUSE

___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

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

2012-08-13 Thread Shawn Landden 
/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);
-if (!p) {
-log_oom();
-return;
-}
+if (!p)
+return (void)log_oom();
 
-j = read_full_file(/etc/localtime, a, l);
 k = read_full_file(p, b, q);
-
 free(p);
 
+j = read_full_file(/etc/localtime, a, l);
+
 if (j  0 || k  0 || l != q || memcmp(a, b, l)) {
 log_warning(/etc/localtime and /etc/timezone out of sync.);
 free(tz.zone);
@@ -174,9 +175,34 @@ static void verify_timezone(void) {
 
 static int read_data(void) {
 int r;
+char *t;
 
 free_data();
 
+r = readlink_malloc(/etc/localtime, t);
+if (r  0) {
+if (r == -EINVAL)
+log_warning(/etc/localtime should be a symbolic link 
to a timezone data file in %s, ZONEINFO_PATH);
+else
+log_warning(Failed to get target of %s: %s, 
/etc/localtime, strerror(-r));
+} else {
+/* we only support the trivial relative link of 
(/etc/)..$ABSOLUTE */
+int rel_link_offset = startswith(t, ..) ? strlen(..) : 0;
+
+if (!startswith(t + rel_link_offset, ZONEINFO_PATH))
+log_warning(/etc/localtime should be a symbolic link 
to a timezone data file in %s, ZONEINFO_PATH);
+else {
+tz.zone = strdup(t + rel_link_offset + 
strlen(ZONEINFO_PATH));
+free(t);
+if (!tz.zone)
+return log_oom();
+
+goto have_timezone;
+}
+}
+
+free(t);
+
 r = read_one_line_file(/etc/timezone, tz.zone);
 if (r  0) {
 if (r != -ENOENT)
@@ -192,6 +218,7 @@ static int read_data(void) {
 #endif
 }
 
+have_timezone:
 if (isempty(tz.zone)) {
 free(tz.zone);
 tz.zone = NULL;
@@ -207,6 +234,7 @@ static int read_data(void) {
 static int write_data_timezone(void) {
 int r = 0;
 char *p;
+struct stat st;
 
 if (!tz.zone) {
 if (unlink(/etc/timezone)  0  errno != ENOENT)
@@ -222,15 +250,17 @@ static int write_data_timezone(void) {
 if (!p)
 return log_oom();
 
-r = symlink_or_copy_atomic(p, /etc/localtime);
+r = symlink_atomic(p, /etc/localtime);
 free(p);
 
 if (r  0)
 return r;
 
-r = write_one_line_file_atomic(/etc/timezone, tz.zone);
-if (r  0)
-return r;
+if (stat(/etc/timezone, st) == 0  S_ISREG(st.st_mode)) {
+r = write_one_line_file_atomic(/etc/timezone, tz.zone);
+if (r  0)
+return r;
+}
 
 return 0;
 }
-- 
1.7.9.5

___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel