Re: [PATCH] cp/mv: xattr support

[Jim Meyering]

Kamil Dudka <address@hidden> wrote:
> New version of patch is attached.
>
> On Tuesday 20 January 2009 17:49:44 Pádraig Brady wrote:
>> > On Tuesday 20 January 2009 11:36:20 Pádraig Brady wrote:
>> >> If SELinux contexts and ACLs are implemented using xattrs,
>> >> does --preserve=xattr copy these also, or does it just copy
>> >> particular namespaces?  Worth clarifying in any case.
> Yes, it does. It is now mentioned in coreutils.texi.
>
>> >> Do we need a require_preserve_xattr?
>> >> I.E. should all these be tristate instead of booleans?
>> >
>> > So you want cp to fail on attr_copy_{file,fd} fail with
>> > option --preserve=xattr, but not with option --preserve=all? No problem I
>> > think.
>>
>> Well I'm not sure. I think it should fail if you
>> explicitly ask for xattr and it can't do it?
> Now it fails if you explicitly ask for xattr and can't do it. If cp is built
> without xattr support, it fails even before file copying.
>
>> >>> diff --git a/tests/misc/xattr b/tests/misc/xattr
>> >>> new file mode 100755
>> >>> index 0000000..2f50134
>> >>> --- /dev/null
>> >>> +++ b/tests/misc/xattr
>> >>> @@ -0,0 +1,80 @@
>> >>> +#!/bin/sh
>> >>> +# Ensure that cp --preserve=xattr and mv preserve extended attributes.
>> >>
>> >> we should add a test for `install` to ensure/make obvious
>> >> it doesn't preserve xattrs.
> Ok, added test for install.
>
>> >>> +# Skip this test if cp was built without xattr support:
>> >>> +grep '^#define USE_XATTR 1' $CONFIG_HEADER > /dev/null ||
>> >>> +  skip_test_ "coreutils built without xattr support"
>> >>
>> >> I'd rather test a binary as it's little less coupled I think:
>> >>
>> >> cp --preserve=xattr --help >/dev/null 2>&1 ||
>> >>   skip_test_ "coreutils built without xattr support"
> Slightly changed to use cp -n instead of cp --help as --help always returns
> EXIT_SUCCESS.

Looks good to me, too.

One nit:
I require a warning-free build, so have eliminate
these warnings by marking the variables as unused:

  cc1: warnings being treated as errors
  copy.c: In function 'copy_attr_error':
  copy.c:135: error: unused parameter 'ctx'
  copy.c: In function 'copy_attr_quote':
  copy.c:147: error: unused parameter 'ctx'
  copy.c: In function 'copy_attr_free':
  copy.c:153: error: unused parameter 'ctx'
  copy.c:153: error: unused parameter 'str'
  make[3]: *** [copy.o] Error 1

I'll fold the following into your patch and test
a little more tomorrow.

Also ran "make distcheck" and fixed the two failures that provoked:
  - a spelling nit: s/filesystem/file system/
  - added the new verror.c to po/POTFILES.in
I'll fold them in too.

>From dbe5371ff6935815e50b0763b54bd7c8fca6de66 Mon Sep 17 00:00:00 2001
From: Jim Meyering <address@hidden>
Date: Tue, 27 Jan 2009 22:55:40 +0100
Subject: [PATCH] mark unused

---
 src/copy.c |   10 ++++++----
 1 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/src/copy.c b/src/copy.c
index 7109a14..85d1fea 100644
--- a/src/copy.c
+++ b/src/copy.c
@@ -1,5 +1,5 @@
 /* copy.c -- core functions for copying files and directories
-   Copyright (C) 89, 90, 91, 1995-2008 Free Software Foundation, Inc.
+   Copyright (C) 89, 90, 91, 1995-2009 Free Software Foundation, Inc.

    This program is free software: you can redistribute it and/or modify
    it under the terms of the GNU General Public License as published by
@@ -132,7 +132,8 @@ is_ancestor (const struct stat *sb, const struct dir_list 
*ancestors)

 #if USE_XATTR
 static void
-copy_attr_error (struct error_context *ctx, char const *fmt, ...)
+copy_attr_error (struct error_context *ctx ATTRIBUTE_UNUSED,
+                char const *fmt, ...)
 {
   int err = errno;
   va_list ap;
@@ -144,13 +145,14 @@ copy_attr_error (struct error_context *ctx, char const 
*fmt, ...)
 }

 static char const *
-copy_attr_quote (struct error_context *ctx, char const *str)
+copy_attr_quote (struct error_context *ctx ATTRIBUTE_UNUSED, char const *str)
 {
   return quote (str);
 }

 static void
-copy_attr_free (struct error_context *ctx, char const *str)
+copy_attr_free (struct error_context *ctx ATTRIBUTE_UNUSED,
+               char const *str ATTRIBUTE_UNUSED)
 {
 }

--
1.6.1.1.374.g0d9d7