On Wed, 2006-08-02 at 10:24 +0300, Sergey Poznyakoff wrote: > James Antill <[EMAIL PROTECTED]> wrote: > > > Hi, I've created support for creating/extracting SELinux/user/root > > xattrs in GNUtar. If you have some free time, I'd appreciate anyone > > looking over the patch and letting me know if anything needs to change > > for inclusion. > > Thank you. After a quick glance it turns out the patch does need something > done before accepting for inclusion:
Thanks for the reply. > 1. All xattrs-specific stuff should be concentrated in one source module > instead of being scattered around 7 source files; I can see why you'd want this for some of the code (xheader_xattr_* for example) ... but, for instance, I'd assume that the code in start_header() and options processing in tar.c should stay where it is? > 2. Proper autoconf machinery should be added to detect the presence of > the special calls it uses (fgetfilecon, flistxattr and a lot of > others) and disable it if any of them is not available; Yeh, that's on my TODO list. > 3. I don't see any reason to create a second (prefix) table in > xheader.c, I'd prefer to expand the functionality of the main table > instead; I can combine the tables with a "is_prefix" type member, I'm not sure this is better though. As the code should probably still run through the table twice in lookup (one for a direct match and once for prefix matches), and ignore the prefix entries in the other functions. But I'm happy to do that, if you want. > 4. The patch is based on 1.15.90 version, current CVS version has moved > ahead since then (in particular, xheader.c was changed), so it would > be advisable to base it on the CVS head. Yeh, I assume that. > > Known "issues": > [...] > > 7. Currently tar defaults to not doing anything when creating an > > archive, so you need to pass --xattrs or --selinux to get all xattrs or > > just SELinux context information in the archive. > > That's OK as well. Would it be ok to have one or both of these on by default, at some point in the future? > > 8. AIUI star only includes the headers when using it's special exustar > > format, the patch I've done for GNUtar just includes it on the else > > clause of: > > > > if (archive_format == V7_FORMAT || numeric_owner_option) > > > > ...which probably needs to be just the former test, or maybe limited to > > certain formats? > > It should be limited to POSIX.1-2001 format only (POSIX_FORMAT). This would mean the default would have to change when you enable that option[1], is there a reason to not allow it with the gnu format? Also to be compatible[2] with star we need to at least extract from ustar typed archives (star's exustar is treated by GNUtar as ustar ... AIUI). What are the downsides to changing the default to be posix -- which is basically what would be happening as xattr usage is leaking into more and more applications. The documentation says that this is planned when full support for posix archives is done, is this close to happening? [1] Or have the horribly ugly UI that star has where you need to specify --xattrs and --format=posix or it'd fail, which seems to be much worse for no real gain. [2] I can't think of a reason we'd want to not allow people to extract star archives they've already made. -- James Antill <[EMAIL PROTECTED]>
signature.asc
Description: This is a digitally signed message part
_______________________________________________ Bug-tar mailing list [email protected] http://lists.gnu.org/mailman/listinfo/bug-tar
