search for: do_internal_lxattrlist

https://bugzilla.redhat.com/show_bug.cgi?id=1811539 Commands including virt-diff which read extended attributes will sometimes fail on NTFS filesystems that are using system compressed. The reason is complex, see comment 5 of the bug linked above. This patch filters out the troublesome xattr. For justification, see the comment I added in patch 3. Patch 1 & 2 are refactoring. I was on the

...r->guestfs_int_xattr_list_val[j].attrval.attrval_val, + vlen = getxattr (path, names[i], + r->guestfs_int_xattr_list_val[i].attrval.attrval_val, vlen); CHROOT_OUT; if (vlen == -1) { @@ -276,7 +305,7 @@ guestfs_int_xattr_list * do_internal_lxattrlist (const char *path, char *const *names) { guestfs_int_xattr_list *ret = NULL; - size_t i, j; + size_t i; size_t k, m, nr_attrs; ssize_t len, vlen; @@ -293,6 +322,7 @@ do_internal_lxattrlist (const char *path, char *const *names) void *newptr; CLEANUP_FREE char *pathname = NUL...

...r->guestfs_int_xattr_list_val[j].attrval.attrval_val, + vlen = getxattr (path, names[i], + r->guestfs_int_xattr_list_val[i].attrval.attrval_val, vlen); CHROOT_OUT; if (vlen == -1) { @@ -276,7 +302,7 @@ guestfs_int_xattr_list * do_internal_lxattrlist (const char *path, char *const *names) { guestfs_int_xattr_list *ret = NULL; - size_t i, j; + size_t i; size_t k, m, nr_attrs; ssize_t len, vlen; @@ -293,6 +319,7 @@ do_internal_lxattrlist (const char *path, char *const *names) void *newptr; CLEANUP_FREE char *pathname = NUL...

v1 here: https://www.redhat.com/archives/libguestfs/2020-March/msg00099.html This one fixes most of the points picked up in review, and does not strdup the strings which should keep down memory usage if that is a concern. Rich.

...error already. */ goto error; - names = split_attr_names (buf, len); + names_unfiltered = split_attr_names (buf, len); + if (names_unfiltered == NULL) + goto error; + names = filter_list (not_hidden_xattr, names_unfiltered); if (names == NULL) goto error; @@ -319,6 +348,7 @@ do_internal_lxattrlist (const char *path, char *const *names) void *newptr; CLEANUP_FREE char *pathname = NULL; CLEANUP_FREE char *buf = NULL; + CLEANUP_FREE /* not string list */ char **attrnames_unfiltered = NULL; CLEANUP_FREE /* not string list */ char **attrnames = NULL; /* Be careful in...

...t char *path, char *list, size_t size), ssize_t *size); +static char *_getxattr (const char *name, const char *path, ssize_t (*getxattr) (const char *path, const char *name, void *value, size_t size), size_t *size_r); guestfs_int_xattr_list * do_getxattrs (const char *path) @@ -448,6 +449,15 @@ do_internal_lxattrlist (const char *path, char *const *names) char * do_getxattr (const char *path, const char *name, size_t *size_r) { + return _getxattr (name, path, getxattr, size_r); +} + +static char * +_getxattr (const char *name, const char *path, + ssize_t (*getxattr) (const char *path, const char *...

...; the comment I added in patch 3. The idea is fine, however I'm slightly concerned about the increased memory usage due to the split + filtering done. Now that I took a look at the code again (wow, last time I touched it was more than 5 years ago), an alternative approach could be: - refactor do_internal_lxattrlist to use _listxattrs - edit/replace in-place the buffer to-be-returned by _listxattrs filtering out the names we do not want This way the rest of the existing code needs almost no changes. -- Pino Toscano

...ror; + } + + CHROOT_IN; + len = listxattr (path, buf, len); + CHROOT_OUT; + if (len == -1) { + reply_with_perror ("listxattr: %s", path); + goto error; + } + + if (size) + *size = len; + return buf; + + error: + free (buf); + return NULL; +} + guestfs_int_xattr_list * do_internal_lxattrlist (const char *path, char *const *names) { -- 1.8.3.1

On Tuesday 07 January 2014 21:04:36 Richard W.M. Jones wrote: > On Tue, Jan 07, 2014 at 04:06:43PM +0100, Pino Toscano wrote: > > Hi, > > > > attached there is a prototype of patch for adding a new > > copy-attributes command. Such command would allow copy the > > attributes of a "file" to> > > another, so for example in guestfish: > >

Add some warnings. Well, the first patch is a miscellaneous change, but patches 2-5 add some warnings. Rich.

In the kernel and qemu communities it is routine for patches that will be backported to stable branches to be posted for review. I'm proposing we do the same for libguestfs stable branches. All of the attached have been tested with 'make check-release'. Rich.