diff options
| -rw-r--r-- | CMakeLists.txt | 12 | ||||
| -rw-r--r-- | tdeio/CMakeLists.txt | 2 | ||||
| -rw-r--r-- | tdeio/tdeio/CMakeLists.txt | 7 | ||||
| -rw-r--r-- | tdeio/tdeio/README.xattr | 55 | ||||
| -rw-r--r-- | tdeio/tdeio/job.cpp | 140 |
5 files changed, 149 insertions, 67 deletions
diff --git a/CMakeLists.txt b/CMakeLists.txt index 84158ef6d..3618f20b7 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -284,13 +284,17 @@ if( NOT HAVE_SYS_UCRED_H ) endif( ) endif( ) check_include_file( "sys/xattr.h" HAVE_SYS_XATTR_H ) + check_include_file( "attr/libattr.h" HAVE_ATTR_LIBATTR_H) if ( HAVE_ATTR_LIBATTR_H ) - check_library_exists( attr attr_copy_file "" HAVE_ATTR__ATTR_COPY_FILE ) -endif( ) -if( HAVE_ATTR__ATTR_COPY_FILE ) - set( DO_USER_XATTR_COPYING "1" ) + message( STATUS "Have attr/libattr.h" ) + check_library_exists( attr attr_copy_file "" HAVE_ATTR__ATTR_COPY_FILE ) + if( HAVE_ATTR__ATTR_COPY_FILE ) + message( STATUS "Have attr_copy_file in libattr" ) + set( USER_XATTR_LIBRARIES attr ) + set( DO_USER_XATTR_COPYING 1 ) message( STATUS "User extended attribute copy support enabled" ) + endif( ) endif( ) check_include_file( "termios.h" HAVE_TERMIOS_H ) diff --git a/tdeio/CMakeLists.txt b/tdeio/CMakeLists.txt index 26bfc53aa..2df71788e 100644 --- a/tdeio/CMakeLists.txt +++ b/tdeio/CMakeLists.txt @@ -92,6 +92,6 @@ tde_add_library( ${target} SHARED VERSION 14.0.0 EMBED kssl-static tdeiocore-static tdesycoca-static kbookmarks-static tdefile-static ${ELFICON_STATIC_LIB} LINK tdeui-shared tdewalletclient-shared - LINK_PRIVATE ltdlc-static tdesu-shared ${LIBR_LIBRARIES} ${ACL_LIBRARIES} + LINK_PRIVATE ltdlc-static tdesu-shared ${LIBR_LIBRARIES} ${ACL_LIBRARIES} ${USER_XATTR_LIBRARIES} DESTINATION ${LIB_INSTALL_DIR} ) diff --git a/tdeio/tdeio/CMakeLists.txt b/tdeio/tdeio/CMakeLists.txt index 91828320a..621101c48 100644 --- a/tdeio/tdeio/CMakeLists.txt +++ b/tdeio/tdeio/CMakeLists.txt @@ -84,6 +84,13 @@ tde_add_library( ${target} STATIC_PIC AUTOMOC SOURCES ${${target}_SRCS} ) +if( DO_USER_XATTR_COPYING ) + add_compile_definitions( DO_USER_XATTR_COPYING ) + if ( HAVE_ATTR__ATTR_COPY_FILE ) + add_compile_definitions( HAVE_ATTR__ATTR_COPY_FILE ) + endif ( HAVE_ATTR__ATTR_COPY_FILE ) +endif( DO_USER_XATTR_COPYING ) + ##### tdesycoca ################################### diff --git a/tdeio/tdeio/README.xattr b/tdeio/tdeio/README.xattr index 50dcaf42b..8761702d2 100644 --- a/tdeio/tdeio/README.xattr +++ b/tdeio/tdeio/README.xattr @@ -9,28 +9,19 @@ source file has been closed and immediately before the source file is deleted (in the case that a move is being done). This code is based on the sample code[4] provided with libattr (examples/copyattr.c). -This code has been in daily use by the author of this patch for just over five -years without any issues. What's missing though is the addition of tests for -xattr and libattr support at configuration/compile time. The code I've added -is enclosed by '#ifdef DO_USER_XATTR_COPYING' statements so a test should be -added that sets this macro[5] if configure-time tests show that xattrs/libattr -are working. I've added a preliminary test to CMakeLists.txt but I'm not sure -it's correct. Alternatively uncomment the '#define DO_USER_XATTR_COPYING 1' -statement at the top of the code to force my code to be enabled. Ensuring that -the cmake test working correctly needs to be done before general release. - This code has been tested on select Ubuntu Linux releases (natty, precise, xenial, and jammy) using x86_64 Linux kernels 3.14.1, 4.18.3, and 5.15.0. -Tested filesystems are ext4 and squashfs. +Tested filesystems are ext4 and squashfs. For ext2/3/4 filesytems the ext_attr +feature must be enabled and the filesystem mounted with the user_xattr option. As far as cross-platform availability goes this code is only known to work on -Linux[6][7]. That said, this code adds functionality that (as far as I'm aware) +Linux[5][6]. That said, this code adds functionality that (as far as I'm aware) isn't currently present in TDE on any of the platforms it runs on. In other words, even if it only runs on Linux in its current state there is no loss of functionality on any other platforms (just a lack of a gaining a feature). In addition, determining the proper location to insert the library call was the hard part with the actual implementation being quite straightforward. Adding -additional backends for other platforms should be pretty easy. +additional backends for other platforms should be pretty easy[7]. To test the code build and install it[8] then run the following from the commandline in e.g. Konsole: @@ -71,17 +62,41 @@ Enjoy! [4] The code that this patch is based on is licensed GPL v2 (or later). -[5] One has to use '#define DO_USER_XATTR_COPYING 1'; specifying instead - '#define DO_USER_XATTR_COPYING' doesn't enable the code (at least on my - system). - -[6] I'm not sure which non-Linux platforms (if any) libattr runs on. +[5] I'm not sure which non-Linux platforms (if any) runs on. It appears that + Cygwin and GNU/Hurd are also supported but this has not been verified. -[7] Another known issue is namespaces. Linux, FreeBSD, and NetBSD support +[6] One potential issue is namespaces. Linux, FreeBSD, and NetBSD support xattrs using the 'user' namespace. Solaris and OSX support user xattrs - but do not support namespaces. + but do not support namespaces. When using libattr, attributes from all + namespaces are copied except those whose names match the patterns listed + in /etc/xattr.conf; other platforms may need special handling. (source: File::ExtAttr(3pm), personal testing) +[7] These files in the tdelibs repository need to be updated when adding + support for a new platform (search for DO_USER_XATTR_COPYING): + + CMakeLists.txt + Add test for any external library used; the library to be linked must + be given in USER_XATTR_LIBRARIES. + + tdeio.tdeio/CMakeLists.txt + Add the new C preprocessor definition (after the existing one for + HAVE_ATTR__ATTR_COPY_FILE). + + tdeio/tdeio/job.cpp + Add new boilerplate code and attribute copy function call (both wrapped + in a conditional checking for the definition set above). + + tdeio/tdeio/README.xattr (this file) + Add additional notes as appropriate. + + Also add any distribution-specific library requirements to the tde-packaging + repository. + [8] Instead of doing a full install one can simply copy the updated library: cp -va "BUILD_DIR/tdeio/libtdeio.so.14.0.0" /opt/trinity/lib/ && ldconfig + + One may also need to run: + + killall tdeio_file diff --git a/tdeio/tdeio/job.cpp b/tdeio/tdeio/job.cpp index 48cc68e3b..839a7bfd6 100644 --- a/tdeio/tdeio/job.cpp +++ b/tdeio/tdeio/job.cpp @@ -95,10 +95,12 @@ extern "C" { #ifdef DO_USER_XATTR_COPYING -//#include <stdio.h> // For stderr, (v)fprintf (already included above) +#ifdef HAVE_ATTR__ATTR_COPY_FILE + +//#include <stdio.h> // For vsnprintf (already included above) //#include <errno.h> // For errno (already included above) -//#include <stdlib.h> // For free (already included above) -//#include <string.h> // For strerror, strdup, and strcmp (already implicitly included above) +//#include <stdlib.h> // For malloc and free (already included above) +//#include <string.h> // For strerror and strncmp (already implicitly included above) #include <stdarg.h> // For va_start and va_end extern "C" { @@ -110,60 +112,107 @@ extern "C" { * Optional error handler for attr_copy_file(). CTX is the error * context passed to attr_copy_file(), ERR is the errno value * that occurred. FMT and the rest are printf style arguments. + * + * No action is taken beyond logging. Errors commonly occur when copying + * files to filesystems that don't support extended attributes (such as + * /tmp on Linux). */ static void error(struct error_context *ctx, const char *fmt, ...) { - va_list ap; - - va_start(ap, fmt); - if (vfprintf(stderr, fmt, ap)) - fprintf(stderr, ": "); - fprintf(stderr, "%s\n", strerror(errno)); - va_end(ap); -} - -/* - * Optional handler for quoting path names in error messages. - * (This is a very stupid example!) - */ -static const char * -quote(struct error_context *ctx, const char *pathname) -{ - char *pn = strdup(pathname), *p; - pathname = strdup(pathname); - for (p = pn; *p != '\0'; p++) - if (*p & 0x80) - *p='?'; - return pn; -} - -static void -quote_free(struct error_context *ctx, const char *name) -{ - free((void *)name); +// Original sample code: +// va_list ap; +// +// va_start(ap, fmt); +// if (vfprintf(stderr, fmt, ap)) +// fprintf(stderr, ": "); +// fprintf(stderr, "%s\n", strerror(errno)); +// va_end(ap); + + va_list ap; + + // Save a copy since it may get clobbered + int our_errno = errno; + + if ( fmt ) { + // Format string was supplied + + // Get length of formatted string (may be 0) + va_start(ap, fmt); + int len = vsnprintf(nullptr, 0, fmt, ap); + va_end(ap); + + if ( len > 0 ) { + // There's a prefix string, output it + char* buffer = (char*)malloc(len+1); + if ( buffer ) { + // Buffer allocated, write the error to it + va_start(ap, fmt); + int written = vsnprintf(&buffer[0], len+1, fmt, ap); + va_end(ap); + if ( written > 0 ) { + // Write message to log + kdDebug(7007) << "Error in attr_copy_file: " << buffer << ": " << strerror(our_errno) << endl; + } else { + // vsnprintf failed or returned an empty string, just log what we can + kdDebug(7007) << "Error in attr_copy_file: " << strerror(our_errno) << endl; + } + } else { + // Buffer allocation failed, just log what we can + kdDebug(7007) << "Error in attr_copy_file: " << strerror(our_errno) << endl; + } + free(buffer); + + } else { + // No prefix string, just log the error code + kdDebug(7007) << "Error in attr_copy_file: " << strerror(our_errno) << endl; + } + } else { + // No format string, just log the error code + kdDebug(7007) << "Error in attr_copy_file: " << strerror(our_errno) << endl; + } } /* * The error context we pass to attr_copy_file(). */ -struct error_context ctx = { error, quote, quote_free }; +struct error_context ctx = { error, NULL, NULL }; /* - * Optional attribute filter for attr_copy_file(). This example - * excludes all attributes other than extended user attributes. + * Optional attribute filter for attr_copy_file(). This example + * excludes all attributes other than extended user attributes + * (those prefixed with "user."). + * + * Returns 0 if the attribute should be copied, non-zero otherwise + * + * If this function is not specified then the default behaviour of + * attr_copy_file is to copy all attributes except for ACLs (as listed + * in /etc/xattr.conf; all patterns that match are skipped). * * This function isn't used by the current code. */ static int is_user_attr(const char *name, struct error_context *ctx) { - // Note that non-Linux systems (including Solaris and Darwin/OSX) may - // use a different naming scheme for user attributes. AFAIK FreeBSD - // and NetBSD do support the "user" namespace. - return strcmp(name, "user.") == 0; + // Note that non-Linux systems (including Solaris and Darwin/OSX) may + // use a different naming scheme for user attributes. AFAIK FreeBSD + // and NetBSD do support the "user" namespace. + + if ( name ) { + // Name is not a null pointer + return strncmp(name, "user.", 5) == 0; + } else { + // Name is a null pointer + return -1; // Not matched + } + + // If this function is not specified in the call to attr_copy_file then the + // default behavior is to use the following heuristic instead (see above): + // return attr_copy_action(name, ctx) == 0; } -#endif /* USE_XATTR */ +#endif /* HAVE_ATTR__ATTR_COPY_FILE */ + +#endif /* DO_USER_XATTR_COPYING */ @@ -2049,11 +2098,18 @@ void FileCopyJob::slotResult( TDEIO::Job *job) m_copyJob = 0; #ifdef DO_USER_XATTR_COPYING +#ifdef HAVE_ATTR__ATTR_COPY_FILE { - // Copy extended attributes (added AKH 2017-11-26) - attr_copy_file(TQFile::encodeName(m_src.path()), TQFile::encodeName(m_dest.path()), NULL, &ctx); + // Copy extended attributes (added AKH 2017-11-26) + // Returned error code (0 on success, -1 on failure) is ignored + int err = attr_copy_file(TQFile::encodeName(m_src.path()), TQFile::encodeName(m_dest.path()), NULL, &ctx); + + // The same but using the attribute name filter function defined above (is_user_attr) + //int err = attr_copy_file(TQFile::encodeName(m_src.path()), TQFile::encodeName(m_dest.path()), is_user_attr, &ctx); } -#endif + +#endif /* HAVE_ATTR__ATTR_COPY_FILE */ +#endif /* DO_USER_XATTR_COPYING */ if (m_move) { |