From fdcb846ff44c3d228bde42f0c6e9790af62f1e01 Mon Sep 17 00:00:00 2001 From: Jaroslav Rohel Date: Thu, 25 Apr 2024 11:17:41 +0200 Subject: [PATCH 1/2] Define and use xattr key to store mtime of file checksum as macro #define XATTR_CHKSUM_MTIME XATTR_CHKSUM_PREFIX "mtime" Before that, the code used: `timestamp_key = g_strconcat(XATTR_CHKSUM_PREFIX, "mtime", NULL);` --- librepo/checksum.c | 7 +++---- librepo/xattr_internal.h | 1 + tests/test_checksum.c | 20 +++++++------------- 3 files changed, 11 insertions(+), 17 deletions(-) diff --git a/librepo/checksum.c b/librepo/checksum.c index 4831ddca1..55432a6d6 100644 --- a/librepo/checksum.c +++ b/librepo/checksum.c @@ -229,19 +229,18 @@ lr_checksum_fd_compare(LrChecksumType type, _cleanup_free_ gchar *timestamp_str = g_strdup_printf("%lli", timestamp); const char *type_str = lr_checksum_type_to_str(type); - _cleanup_free_ gchar *timestamp_key = g_strconcat(XATTR_CHKSUM_PREFIX, "mtime", NULL); _cleanup_free_ gchar *checksum_key = g_strconcat(XATTR_CHKSUM_PREFIX, type_str, NULL); if (caching && timestamp != -1) { // Load cached checksum if enabled and used char buf[256]; ssize_t attr_size; - attr_size = FGETXATTR(fd, timestamp_key, &buf, sizeof(buf)-1); + attr_size = FGETXATTR(fd, XATTR_CHKSUM_MTIME, &buf, sizeof(buf)-1); if (attr_size != -1) { buf[attr_size] = 0; // check that mtime stored in xattr is the same as timestamp if (strcmp(timestamp_str, buf) == 0) { - g_debug("%s: Using mtime cached in xattr: [%s] %s", __func__, timestamp_key, buf); + g_debug("%s: Using mtime cached in xattr: [%s] %s", __func__, XATTR_CHKSUM_MTIME, buf); attr_size = FGETXATTR(fd, checksum_key, &buf, sizeof(buf)-1); if (attr_size != -1) { buf[attr_size] = 0; @@ -279,7 +278,7 @@ lr_checksum_fd_compare(LrChecksumType type, if (caching && *matches && timestamp != -1) { // Store timestamp and checksum as extended file attribute if caching is enabled - FSETXATTR(fd, timestamp_key, timestamp_str, strlen(timestamp_str), 0); + FSETXATTR(fd, XATTR_CHKSUM_MTIME, timestamp_str, strlen(timestamp_str), 0); FSETXATTR(fd, checksum_key, checksum, strlen(checksum), 0); } diff --git a/librepo/xattr_internal.h b/librepo/xattr_internal.h index d0f465d9a..e2c66b37a 100644 --- a/librepo/xattr_internal.h +++ b/librepo/xattr_internal.h @@ -22,6 +22,7 @@ #define __LR_XATTR_INTERNAL_H__ #define XATTR_CHKSUM_PREFIX "user.Librepo.checksum." +#define XATTR_CHKSUM_MTIME XATTR_CHKSUM_PREFIX "mtime" #if __APPLE__ diff --git a/tests/test_checksum.c b/tests/test_checksum.c index efac88b78..f44a8ed6f 100644 --- a/tests/test_checksum.c +++ b/tests/test_checksum.c @@ -102,7 +102,6 @@ START_TEST(test_cached_checksum_matches) struct stat st; char buf[256]; GError *tmp_err = NULL; - gchar *timestamp_key = g_strconcat(XATTR_CHKSUM_PREFIX, "mtime", NULL); gchar *checksum_key = g_strconcat(XATTR_CHKSUM_PREFIX, "sha256", NULL); gchar *mtime_str = NULL; @@ -113,7 +112,7 @@ START_TEST(test_cached_checksum_matches) fclose(f); // Assert no cached checksum exists - attr_ret = GETXATTR(filename, timestamp_key, &buf, sizeof(buf)-1); + attr_ret = GETXATTR(filename, XATTR_CHKSUM_MTIME, &buf, sizeof(buf)-1); ck_assert(attr_ret == -1); // Cached timestamp should not exists attr_ret = GETXATTR(filename, checksum_key, &buf, sizeof(buf)-1); ck_assert(attr_ret == -1); // Cached checksum should not exists @@ -155,7 +154,7 @@ START_TEST(test_cached_checksum_matches) timestamp *= 1000000000; //convert sec timestamp to nanosec timestamp timestamp += st.st_mtim.tv_nsec; mtime_str = g_strdup_printf("%lli", timestamp); - attr_ret = GETXATTR(filename, timestamp_key, &buf, sizeof(buf)-1); + attr_ret = GETXATTR(filename, XATTR_CHKSUM_MTIME, &buf, sizeof(buf)-1); ck_assert(attr_ret != -1); buf[attr_ret] = 0; ck_assert_str_eq(buf, mtime_str); @@ -176,7 +175,6 @@ START_TEST(test_cached_checksum_matches) exit_label: lr_free(filename); - lr_free(timestamp_key); lr_free(checksum_key); lr_free(mtime_str); } @@ -192,7 +190,6 @@ START_TEST(test_cached_checksum_value) static char *expected = "d78931fcf2660108eec0d6674ecb4e02401b5256a6b5ee82527766ef6d198c67"; char buf[256]; GError *tmp_err = NULL; - gchar *timestamp_key = g_strconcat(XATTR_CHKSUM_PREFIX, "mtime", NULL); gchar *checksum_key = g_strconcat(XATTR_CHKSUM_PREFIX, "sha256", NULL); gchar *mtime_str = NULL; gchar *calculated = NULL; @@ -204,7 +201,7 @@ START_TEST(test_cached_checksum_value) fclose(f); // Assert no cached checksum exists - attr_ret = GETXATTR(filename, timestamp_key, &buf, sizeof(buf)-1); + attr_ret = GETXATTR(filename, XATTR_CHKSUM_MTIME, &buf, sizeof(buf)-1); ck_assert(attr_ret == -1); // Cached timestamp should not exists attr_ret = GETXATTR(filename, checksum_key, &buf, sizeof(buf)-1); ck_assert(attr_ret == -1); // Cached checksum should not exists @@ -229,14 +226,13 @@ START_TEST(test_cached_checksum_value) // Assert no cached checksum exists // This assumes issue #235 is unresolved. Once it is, this code // should fail and the test will need updated. - attr_ret = GETXATTR(filename, timestamp_key, &buf, sizeof(buf)-1); + attr_ret = GETXATTR(filename, XATTR_CHKSUM_MTIME, &buf, sizeof(buf)-1); ck_assert(attr_ret == -1); // Cached timestamp should not exists attr_ret = GETXATTR(filename, checksum_key, &buf, sizeof(buf)-1); ck_assert(attr_ret == -1); // Cached checksum should not exists lr_free(calculated); g_free(filename); - g_free(timestamp_key); g_free(checksum_key); lr_free(mtime_str); } @@ -249,7 +245,6 @@ START_TEST(test_cached_checksum_clear) ssize_t attr_ret; char *filename; char buf[256]; - gchar *timestamp_key = g_strconcat(XATTR_CHKSUM_PREFIX, "mtime", NULL); gchar *checksum_key = g_strconcat(XATTR_CHKSUM_PREFIX, "sha256", NULL); const char *other_key = "user.Other.Attribute"; const char *value = "some value"; @@ -262,7 +257,7 @@ START_TEST(test_cached_checksum_clear) // set extended attributes fd = open(filename, O_RDONLY); ck_assert_int_ge(fd, 0); - attr_ret = FSETXATTR(fd, timestamp_key, value, strlen(value), 0); + attr_ret = FSETXATTR(fd, XATTR_CHKSUM_MTIME, value, strlen(value), 0); if (attr_ret == -1) { if (errno == ENOTSUP) { goto cleanup; @@ -275,7 +270,7 @@ START_TEST(test_cached_checksum_clear) ck_assert(attr_ret != -1); // verify that xattrs are set - attr_ret = GETXATTR(filename, timestamp_key, &buf, sizeof(buf)); + attr_ret = GETXATTR(filename, XATTR_CHKSUM_MTIME, &buf, sizeof(buf)); ck_assert(attr_ret != -1); attr_ret = GETXATTR(filename, checksum_key, &buf, sizeof(buf)); ck_assert(attr_ret != -1); @@ -285,7 +280,7 @@ START_TEST(test_cached_checksum_clear) lr_checksum_clear_cache(fd); // verify that checksum xattrs are removed - attr_ret = GETXATTR(filename, timestamp_key, &buf, sizeof(buf)); + attr_ret = GETXATTR(filename, XATTR_CHKSUM_MTIME, &buf, sizeof(buf)); ck_assert(attr_ret == -1); attr_ret = GETXATTR(filename, checksum_key, &buf, sizeof(buf)); ck_assert(attr_ret == -1); @@ -295,7 +290,6 @@ START_TEST(test_cached_checksum_clear) cleanup: close(fd); g_free(filename); - g_free(timestamp_key); g_free(checksum_key); } END_TEST From 9cbda3ce4c132213aecd4af3cfd17c7804b3c75f Mon Sep 17 00:00:00 2001 From: Jaroslav Rohel Date: Thu, 25 Apr 2024 12:23:51 +0200 Subject: [PATCH 2/2] Do not work with xattrs after finding out that they are not supported --- librepo/checksum.c | 28 +++++++++++++++++++--------- 1 file changed, 19 insertions(+), 9 deletions(-) diff --git a/librepo/checksum.c b/librepo/checksum.c index 55432a6d6..199e40e84 100644 --- a/librepo/checksum.c +++ b/librepo/checksum.c @@ -256,6 +256,10 @@ lr_checksum_fd_compare(LrChecksumType type, // timestamp stored in xattr is different => checksums are no longer valid lr_checksum_clear_cache(fd); } + } else if (errno == ENOTSUP) { + // Extended attributes are not supported by the filesystem, or are disabled. + // Disable the use of extended attributes for cache. + caching = FALSE; } } @@ -293,22 +297,28 @@ lr_checksum_fd_compare(LrChecksumType type, void lr_checksum_clear_cache(int fd) { - char *xattrs = NULL; - ssize_t xattrs_len; - ssize_t bytes_read; - const char *attr; - ssize_t prefix_len = strlen(XATTR_CHKSUM_PREFIX); + // Remove the extended attribute containing mtime. + if (FREMOVEXATTR(fd, XATTR_CHKSUM_MTIME) == -1) { + if (errno == ENOTSUP) { + // Extended attributes are not supported by the filesystem, or are disabled. + return; + } + } - xattrs_len = FLISTXATTR(fd, NULL, 0); + // Remove the other extended attributes used by librepo to store the file checksum. + // Librepo does not need this to work properly. Deleting the mtime attribute is sufficient. + // We're just cleaning up. + ssize_t xattrs_len = FLISTXATTR(fd, NULL, 0); if (xattrs_len <= 0) { return; } - xattrs = lr_malloc(xattrs_len); - bytes_read = FLISTXATTR(fd, xattrs, xattrs_len); + char *xattrs = lr_malloc(xattrs_len); + ssize_t bytes_read = FLISTXATTR(fd, xattrs, xattrs_len); if (bytes_read < 0) { goto cleanup; } - attr = xattrs; + ssize_t prefix_len = strlen(XATTR_CHKSUM_PREFIX); + const char *attr = xattrs; while (attr < xattrs + xattrs_len) { if (strncmp(XATTR_CHKSUM_PREFIX, attr, prefix_len) == 0) { FREMOVEXATTR(fd, attr);