[hacktree] Actually checksum xattr values, not just names

[Colin Walters <walters src gnome org>]

commit 05c35f2cf7ed7e6cdf0cd84a6bb1a381faa46584
Author: Colin Walters <walters verbum org>
Date:   Sat Oct 15 03:03:51 2011 -0400

    Actually checksum xattr values, not just names
    
    We need to call lgetxattr() and not just llistxattr().

 src/libhacktree/hacktree-core.c |  131 ++++++++++++++++++++++-----------------
 src/libhacktree/hacktree-core.h |   19 ++++--
 src/libhacktree/hacktree-repo.c |   17 +++---
 tests/t0005-nested-tree.sh      |    4 +-
 4 files changed, 97 insertions(+), 74 deletions(-)
---
diff --git a/src/libhacktree/hacktree-core.c b/src/libhacktree/hacktree-core.c
index 165dfe7..9817c85 100644
--- a/src/libhacktree/hacktree-core.c
+++ b/src/libhacktree/hacktree-core.c
@@ -61,16 +61,64 @@ canonicalize_xattrs (char *xattr_string, size_t len)
   return g_string_free (result, FALSE);
 }
 
-gboolean
-hacktree_get_xattrs_for_directory (const char *path,
-                                   char      **out_xattrs, /* out */
-                                   gsize      *out_len, /* out */
-                                   GError **error)
+static gboolean
+read_xattr_name_array (const char *path,
+                       const char *xattrs,
+                       size_t      len,
+                       GVariantBuilder *builder,
+                       GError  **error)
 {
   gboolean ret = FALSE;
-  char *xattrs = NULL;
+  const char *p;
+
+  p = xattrs;
+  while (p < xattrs+len)
+    {
+      ssize_t bytes_read;
+      char *buf;
+
+      bytes_read = lgetxattr (path, p, NULL, 0);
+      if (bytes_read < 0)
+        {
+          ht_util_set_error_from_errno (error, errno);
+          goto out;
+        }
+      if (bytes_read == 0)
+        continue;
+
+      buf = g_malloc (bytes_read);
+      if (lgetxattr (path, p, buf, bytes_read) < 0)
+        {
+          ht_util_set_error_from_errno (error, errno);
+          g_free (buf);
+          goto out;
+        }
+      
+      g_variant_builder_add (builder, "(@ay ay)",
+                             g_variant_new_bytestring (p),
+                             g_variant_new_fixed_array (G_VARIANT_TYPE ("y"), buf, bytes_read, 1));
+
+      g_free (buf);
+      p = p + strlen (p) + 1;
+    }
+  
+  ret = TRUE;
+ out:
+  return ret;
+}
+
+GVariant *
+hacktree_get_xattrs_for_path (const char *path,
+                              GError    **error)
+{
+  GVariant *ret = NULL;
+  GVariantBuilder builder;
+  char *xattr_names = NULL;
+  char *xattr_names_canonical = NULL;
   ssize_t bytes_read;
 
+  g_variant_builder_init (&builder, G_VARIANT_TYPE ("a(ayay)"));
+
   bytes_read = llistxattr (path, NULL, 0);
 
   if (bytes_read < 0)
@@ -83,27 +131,25 @@ hacktree_get_xattrs_for_directory (const char *path,
     }
   else if (bytes_read > 0)
     {
-      xattrs = g_malloc (bytes_read);
-      if (!llistxattr (path, xattrs, bytes_read))
+      const char *p;
+      xattr_names = g_malloc (bytes_read);
+      if (llistxattr (path, xattr_names, bytes_read) < 0)
         {
           ht_util_set_error_from_errno (error, errno);
-          g_free (xattrs);
-          xattrs = NULL;
           goto out;
         }
-
-      *out_xattrs = canonicalize_xattrs (xattrs, bytes_read);
-      *out_len = (gsize)bytes_read;
-    }
-  else
-    {
-      *out_xattrs = NULL;
-      *out_len = 0;
+      xattr_names_canonical = canonicalize_xattrs (xattr_names, bytes_read);
+      
+      if (!read_xattr_name_array (path, xattr_names_canonical, bytes_read, &builder, error))
+        goto out;
     }
 
-  ret = TRUE;
+  ret = g_variant_builder_end (&builder);
  out:
-  g_free (xattrs);
+  if (!ret)
+    g_variant_builder_clear (&builder);
+  g_free (xattr_names);
+  g_free (xattr_names_canonical);
   return ret;
 }
 
@@ -117,8 +163,7 @@ hacktree_stat_and_checksum_file (int dir_fd, const char *path,
   GChecksum *content_and_meta_sha256 = NULL;
   char *stat_string = NULL;
   ssize_t bytes_read;
-  char *xattrs = NULL;
-  char *xattrs_canonicalized = NULL;
+  GVariant *xattrs = NULL;
   int fd = -1;
   DIR *temp_dir = NULL;
   char *basename = NULL;
@@ -159,39 +204,9 @@ hacktree_stat_and_checksum_file (int dir_fd, const char *path,
     }
 
   stat_string = stat_to_string (&stbuf);
-
-  /* FIXME - Add llistxattrat */
-  if (!S_ISLNK(stbuf.st_mode))
-    bytes_read = flistxattr (fd, NULL, 0);
-  else
-    bytes_read = llistxattr (path, NULL, 0);
-
-  if (bytes_read < 0)
-    {
-      if (errno != ENOTSUP)
-        {
-          ht_util_set_error_from_errno (error, errno);
-          goto out;
-        }
-    }
-  else if (bytes_read > 0)
-    {
-      gboolean tmp;
-      xattrs = g_malloc (bytes_read);
-      /* FIXME - Add llistxattrat */
-      if (!S_ISLNK(stbuf.st_mode))
-        tmp = flistxattr (fd, xattrs, bytes_read);
-      else
-        tmp = llistxattr (path, xattrs, bytes_read);
-          
-      if (!tmp)
-        {
-          ht_util_set_error_from_errno (error, errno);
-          goto out;
-        }
-
-      xattrs_canonicalized = canonicalize_xattrs (xattrs, bytes_read);
-    }
+  xattrs = hacktree_get_xattrs_for_path (path, error);
+  if (!xattrs)
+    goto out;
 
   content_sha256 = g_checksum_new (G_CHECKSUM_SHA256);
  
@@ -235,7 +250,7 @@ hacktree_stat_and_checksum_file (int dir_fd, const char *path,
   content_and_meta_sha256 = g_checksum_copy (content_sha256);
 
   g_checksum_update (content_and_meta_sha256, (guint8*)stat_string, strlen (stat_string));
-  g_checksum_update (content_and_meta_sha256, (guint8*)xattrs_canonicalized, strlen (stat_string));
+  g_checksum_update (content_and_meta_sha256, (guint8*)g_variant_get_data (xattrs), g_variant_get_size (xattrs));
 
   *out_stbuf = stbuf;
   *out_checksum = content_and_meta_sha256;
@@ -248,8 +263,8 @@ hacktree_stat_and_checksum_file (int dir_fd, const char *path,
   g_free (symlink_target);
   g_free (basename);
   g_free (stat_string);
-  g_free (xattrs);
-  g_free (xattrs_canonicalized);
+  if (xattrs)
+    g_variant_unref (xattrs);
   if (content_sha256)
     g_checksum_free (content_sha256);
 
diff --git a/src/libhacktree/hacktree-core.h b/src/libhacktree/hacktree-core.h
index 5102039..4f06ae5 100644
--- a/src/libhacktree/hacktree-core.h
+++ b/src/libhacktree/hacktree-core.h
@@ -36,11 +36,18 @@ typedef enum {
 typedef enum {
   HACKTREE_SERIALIZED_TREE_VARIANT = 1,
   HACKTREE_SERIALIZED_COMMIT_VARIANT = 2,
-  HACKTREE_SERIALIZED_DIRMETA_VARIANT = 3
+  HACKTREE_SERIALIZED_DIRMETA_VARIANT = 3,
+  HACKTREE_SERIALIZED_XATTR_VARIANT = 4
 } HacktreeSerializedVariantType;
 
 #define HACKTREE_SERIALIZED_VARIANT_FORMAT "(uv)"
 
+/*
+ * xattr objects:
+ * a(ayay) - array of (name, value) pairs, both binary data, though name is a bytestring
+ */
+#define HACKTREE_XATTR_GVARIANT_FORMAT "a(ayay)"
+
 #define HACKTREE_DIR_META_VERSION 0
 /*
  * dirmeta objects:
@@ -48,9 +55,9 @@ typedef enum {
  * u - uid
  * u - gid
  * u - mode
- * ay - xattrs
+ * a(ayay) - xattrs
  */
-#define HACKTREE_DIRMETA_GVARIANT_FORMAT "(uuuuay)"
+#define HACKTREE_DIRMETA_GVARIANT_FORMAT "(uuuua(ayay))"
 
 #define HACKTREE_TREE_VERSION 0
 /*
@@ -75,10 +82,8 @@ typedef enum {
  */
 #define HACKTREE_COMMIT_GVARIANT_FORMAT "(ua{sv}sssts)"
 
-gboolean   hacktree_get_xattrs_for_directory (const char *path,
-                                              char      **out_xattrs,
-                                              gsize      *out_len,
-                                              GError    **error);
+GVariant *hacktree_get_xattrs_for_path (const char *path,
+                                        GError    **error);
 
 gboolean hacktree_stat_and_checksum_file (int dirfd, const char *path,
                                           GChecksum **out_checksum,
diff --git a/src/libhacktree/hacktree-repo.c b/src/libhacktree/hacktree-repo.c
index 6541b74..2469b35 100644
--- a/src/libhacktree/hacktree-repo.c
+++ b/src/libhacktree/hacktree-repo.c
@@ -402,7 +402,7 @@ import_directory_meta (HacktreeRepo  *self,
   struct stat stbuf;
   GChecksum *ret_checksum = NULL;
   GVariant *dirmeta = NULL;
-  char *xattrs = NULL;
+  GVariant *xattrs = NULL;
   gsize xattr_len;
 
   if (lstat (path, &stbuf) < 0)
@@ -418,16 +418,17 @@ import_directory_meta (HacktreeRepo  *self,
       goto out;
     }
 
-  if (!hacktree_get_xattrs_for_directory (path, &xattrs, &xattr_len, error))
+  xattrs = hacktree_get_xattrs_for_path (path, error);
+  if (!xattrs)
     goto out;
 
-  dirmeta = g_variant_new ("(uuuu ay)",
+  dirmeta = g_variant_new ("(uuuu a(ayay))",
                            HACKTREE_DIR_META_VERSION,
                            (guint32)stbuf.st_uid,
                            (guint32)stbuf.st_gid,
                            (guint32)(stbuf.st_mode & (S_ISUID|S_ISGID|S_ISVTX|S_IRWXU|S_IRWXG|S_IRWXO)),
-                           g_variant_new_fixed_array (G_VARIANT_TYPE ("y"),
-                                                      xattrs, xattr_len, 1));
+                           xattrs);
+  xattrs = NULL; /* was floating */
   g_variant_ref_sink (dirmeta);
 
   if (!import_gvariant_object (self, HACKTREE_SERIALIZED_DIRMETA_VARIANT, 
@@ -448,7 +449,8 @@ import_directory_meta (HacktreeRepo  *self,
       *out_checksum = ret_checksum;
       *out_variant = dirmeta;
     }
-  g_free (xattrs);
+  if (xattrs)
+    g_variant_unref (xattrs);
   return ret;
 }
 
@@ -1473,10 +1475,9 @@ checkout_one_directory (HacktreeRepo  *self,
 
   dest_path = g_build_filename (destination, dirname, NULL);
       
-  g_variant_get (dir->meta_data, "(uuuu ay)",
+  g_variant_get (dir->meta_data, "(uuuu a(ayay))",
                  &version, &uid, &gid, &mode,
                  &xattr_variant);
-  xattrs = g_variant_get_fixed_array (xattr_variant, &xattr_len, 1);
 
   if (mkdir (dest_path, (mode_t)mode) < 0)
     {
diff --git a/tests/t0005-nested-tree.sh b/tests/t0005-nested-tree.sh
index 76952bc..f7f5182 100755
--- a/tests/t0005-nested-tree.sh
+++ b/tests/t0005-nested-tree.sh
@@ -22,7 +22,7 @@ set -e
 
 . libtest.sh
 
-echo '1..3'
+echo '1..5'
 
 setup_test_repository2
 echo 'ok setup'
@@ -30,8 +30,10 @@ hacktree checkout $ht_repo HEAD $test_tmpdir/checkout2-head
 echo 'ok checkout cmd'
 cd $test_tmpdir/checkout2-head
 assert_has_file firstfile
+echo 'ok checkout firstfile'
 assert_has_file baz/cow
 assert_has_file baz/saucer
+echo 'ok checkout baz (2)'
 assert_has_file baz/deeper/ohyeah
 assert_has_file baz/another/y
 echo 'ok checkout verify exists'