[wip patch] new zsh/attr module

[wip patch] new zsh/attr module

[Mikael Magnusson]

Hi,

in my .zshrc I have this short function:
#usage, *(e:fattr name:) or *(e:fattr name value:)
function fattr() {
   local val="$(getfattr -n user.$1 --only-values $REPLY 2>/dev/null)"
   [[ -n "$val" && ( -z "$2" || "$val" =~ "$2" ) ]]
}

The problem is it takes a long time when you run it on some significant 
set of files since it forks for every file (~10 seconds for hundreds of 
files). So I cobbled together this module to make three builtins, 
zgetattr, zsetattr and zdelattr.

#usage, *(e:fattr name:) or *(e:fattr name value:)
function fattr() {
   local val
   zgetattr $REPLY user.$1 val 2>/dev/null
   [[ -n "$val" && ( -z "$2" || "$val" =~ "$2" ) ]]
}

This runs in 280ms for the same set of files.

I'm not sure if I should mention it being copied from cap.c since pretty 
much only the skeleton remains. I guess I would have to write some 
documentation too. The builtins should probably handle more than one file 
and parse options in a better way. A builtin for listing attrs on a file 
would be useful too (could at least be used for completion of the second 
argument :) ). Maybe the module should be called xattr instead of just 
attr? I also didn't bother checking what happens when the system doesn't 
support xattrs or doesn't have the includes. I guess something similar to 
what db_gdbm.mdd does is needed? I noticed just now that I was lazy and 
used the ?: extension so that's something to fix too.

Do I need to nul terminate strings I give to metafy() and/or setsparam()?

I think the AC_CHECK_LIB isn't exactly right either, it adds a second -lc 
to $LIBS.

diff --git a/Src/Modules/.distfiles b/Src/Modules/.distfiles
index 40d3114..9231cec 100644
--- a/Src/Modules/.distfiles
+++ b/Src/Modules/.distfiles
@@ -2,6 +2,8 @@ DISTFILES_SRC='
  .cvsignore
  .distfiles
  .exrc
+attr.mdd
+attr.c
  cap.mdd
  cap.c
  clone.mdd
diff --git a/Src/Modules/attr.c b/Src/Modules/attr.c
new file mode 100644
index 0000000..fc9c70a
--- /dev/null
+++ b/Src/Modules/attr.c
@@ -0,0 +1,140 @@
+/*
+ * attr.c - extended attributes (xattr) manipulation
+ * (based on cap.c)
+ *
+ * This file is part of zsh, the Z shell.
+ *
+ * Copyright (c) 2009 Mikael Magnusson
+ * Copyright (c) 1997 Andrew Main
+ * All rights reserved.
+ *
+ * Permission is hereby granted, without written agreement and without
+ * license or royalty fees, to use, copy, modify, and distribute this
+ * software and to distribute modified versions of this software for any
+ * purpose, provided that the above copyright notice and the following
+ * two paragraphs appear in all copies of this software.
+ *
+ * In no event shall Mikael Magnusson or the Zsh Development Group be liable
+ * to any party for direct, indirect, special, incidental, or consequential
+ * damages arising out of the use of this software and its documentation,
+ * even if Andrew Main and the Zsh Development Group have been advised of
+ * the possibility of such damage.
+ *
+ * Mikael Magnusson and the Zsh Development Group specifically disclaim any
+ * warranties, including, but not limited to, the implied warranties of
+ * merchantability and fitness for a particular purpose.  The software
+ * provided hereunder is on an "as is" basis, and Mikael Magnusson and the
+ * Zsh Development Group have no obligation to provide maintenance,
+ * support, updates, enhancements, or modifications.
+ *
+ */
+
+#include "attr.mdh"
+#include "attr.pro"
+
+#include <sys/types.h>
+#include <sys/xattr.h>
+
+static int
+bin_getattr(char *nam, char **argv, UNUSED(Options ops), UNUSED(int func))
+{
+    int ret = 0;
+    int len;
+    char value[256];
+
+    if (listxattr(*argv, NULL, 0) > 0) {
+        if (0 < (len = getxattr(*argv, *(argv+1), value, 255))) {
+            if (len < 256) {
+                value[len] = '\0';
+                setsparam(*(argv+2) ?: "REPLY", metafy(value, len, META_DUP));
+            }
+        } else {
+            zwarnnam(nam, "%s: %e", *argv, errno);
+            ret = 1;
+        }
+    }
+    return ret;
+}
+
+static int
+bin_setattr(char *nam, char **argv, UNUSED(Options ops), UNUSED(int func))
+{
+    int ret = 0;
+ 
+    if (setxattr(*argv, *(argv+1), *(argv+2), strlen(*(argv+2)), 0)) {
+        zwarnnam(nam, "%s: %e", *argv, errno);
+        ret = 1;
+    }
+    return ret;
+}
+
+static int
+bin_delattr(char *nam, char **argv, UNUSED(Options ops), UNUSED(int func))
+{
+    int ret = 0;
+ 
+    if (removexattr(*argv, *(argv+1))) {
+        zwarnnam(nam, "%s: %e", *argv, errno);
+        ret = 1;
+    }
+    return ret;
+}
+
+/* module paraphernalia */
+
+static struct builtin bintab[] = {
+    BUILTIN("zgetattr", 0, bin_getattr, 2, 3, 0, NULL, NULL),
+    BUILTIN("zsetattr", 0, bin_setattr, 3, 3, 0, NULL, NULL),
+    BUILTIN("zdelattr", 0, bin_delattr, 2, 2, 0, NULL, NULL),
+};
+
+static struct features module_features = {
+    bintab, sizeof(bintab)/sizeof(*bintab),
+    NULL, 0,
+    NULL, 0,
+    NULL, 0,
+    0
+};
+
+/**/
+int
+setup_(UNUSED(Module m))
+{
+    return 0;
+}
+
+/**/
+int
+features_(Module m, char ***features)
+{
+    *features = featuresarray(m, &module_features);
+    return 0;
+}
+
+/**/
+int
+enables_(Module m, int **enables)
+{
+    return handlefeatures(m, &module_features, enables);
+}
+
+/**/
+int
+boot_(UNUSED(Module m))
+{
+    return 0;
+}
+
+/**/
+int
+cleanup_(Module m)
+{
+    return setfeatureenables(m, &module_features, NULL);
+}
+
+/**/
+int
+finish_(UNUSED(Module m))
+{
+    return 0;
+}
diff --git a/Src/Modules/attr.mdd b/Src/Modules/attr.mdd
new file mode 100644
index 0000000..fc4f7b9
--- /dev/null
+++ b/Src/Modules/attr.mdd
@@ -0,0 +1,7 @@
+name=zsh/attr
+link=dynamic
+load=no
+
+autofeatures="b:zgetattr b:zsetattr b:zdelattr"
+
+objects="attr.o"
diff --git a/configure.ac b/configure.ac
index 50658e5..fe3f229 100644
--- a/configure.ac
+++ b/configure.ac
@@ -852,6 +852,9 @@ if test x$gdbm != xno; then
    AC_CHECK_LIB(gdbm, gdbm_open)
  fi

+AC_CHECK_HEADERS(sys/xattr.h)
+AC_CHECK_LIB(c, getxattr)
+
  dnl --------------
  dnl CHECK TYPEDEFS
  dnl --------------

--
Mikael Magnusson

Re: [wip patch] new zsh/attr module

[Mikael Magnusson]

2009/2/26 Mikael Magnusson <mikachu@gmail.com>:
> The problem is it takes a long time when you run it on some significant set
> of files since it forks for every file (~10 seconds for hundreds of files).
> So I cobbled together this module to make three builtins, zgetattr, zsetattr
> and zdelattr.

The number of files in the above test was 3300, not hundreds.

-- 
Mikael Magnusson

Re: [wip patch] new zsh/attr module

[Mikael Magnusson]

2009/2/26 Mikael Magnusson <mikachu@gmail.com>:
> Hi,

> +static int
> +bin_getattr(char *nam, char **argv, UNUSED(Options ops), UNUSED(int func))
> +{
> +    int ret = 0;
> +    int len;
> +    char value[256];
> +
> +    if (listxattr(*argv, NULL, 0) > 0) {
> +        if (0 < (len = getxattr(*argv, *(argv+1), value, 255))) {

It seems these *argv and the ones below need to be wrapped in unmeta()
to work with utf-8 filenames/values. Obviously I can't use unmeta()
twice in one function call though, can I call unmetafy() on the argv
values or will something be sad then? Can the length grow when I
unmetafy so I would need to alloc more space?

I also note the cap.c file doesn't unmeta(fy) its arguments so it
probably also doesn't work, but I don't have cap stuff so can't test.

Also, in unmeta() would putting a break; in the initial loop help? ie
    for (t = file_name; *t; t++) {
	if (*t == Meta) {
	    meta = 1;
+           break;
        }
    }

(or I suppose you could have *t && !meta)

My impression from looking at the code is that only metafy()ing can
grow the string, so it should be safe to just unmetafy() the strings,
assuming nothing breaks from me modifying the argv strings?

-- 
Mikael Magnusson

Re: [wip patch] new zsh/attr module

[Peter Stephenson]

On Thu, 26 Feb 2009 22:55:47 +0100 (CET)
Mikael Magnusson <mikachu@gmail.com> wrote:
> So I cobbled together this module to make three builtins, 
> zgetattr, zsetattr and zdelattr.
> 
> #usage, *(e:fattr name:) or *(e:fattr name value:)
> function fattr() {
>    local val
>    zgetattr $REPLY user.$1 val 2>/dev/null
>    [[ -n "$val" && ( -z "$2" || "$val" =~ "$2" ) ]]
> }

This looks useful...

> I'm not sure if I should mention it being copied from cap.c since pretty 
> much only the skeleton remains.

I think not.

> I guess I would have to write some documentation too.

Yes.

> The builtins should probably handle more than one file

Right, then you'd need to use arrays to return values.  There's
an argument for an option to put name/value pairs into associative arrays;
that doesn't give you anything fundamentally new but it does prevent you
having to loop over a file name and an attribute array at the same time
which is potentially slow.

> and parse options in a better way.

I think the optional parameter argument is OK; however, adding options is
trivial since you're going through the standard builtin handling code.

> A builtin for listing attrs on a file
> would be useful too (could at least be used for completion of the second
> argument :) ).

Yes, and in this case a missing parameter should just output to standard
output.  I'm in two minds as to whether that would be better with zgetattr
than defaulting to REPLY; it is more general since you can always specify
REPLY if you want to use it (and with more than one file it would be
"reply" anyway), and if the listing command does this it might be more
consistent for the get command to do so.

> Maybe the module should be called xattr instead of just attr?

I'm not particularly bothered either way.  Too many x's and z's is a bit
ugly.  However, there are multiple attribute systems and there's an
argument for being specific.

> I also didn't bother checking what happens when the system doesn't
> support xattrs or doesn't have the includes. I guess something similar to
> what db_gdbm.mdd does is needed? I noticed just now that I was lazy and
> used the ?: extension so that's something to fix too.

Yes, you need to conditionalise linking by putting some configure
(i.e. portable shell code) tests into link=`...` in the .mdd file.
You should probably test both that the function and the header exist.

> Do I need to nul terminate strings I give to metafy() and/or setsparam()?

To metafy(), no, since there's an explicit length and the pre-metafied
input may include NULs within the string (that's one of the points of
metafication); to setsparam(), yes, since that's a metafied NUL-terminated
string.

> I think the AC_CHECK_LIB isn't exactly right either, it adds a second -lc 
> to $LIBS.

You should just add the getxattr test to the AC_CHECK_FUNCS lib.


> It seems these *argv and the ones below need to be wrapped in unmeta()
> to work with utf-8 filenames/values.

Yes, indeed:  internal values are nearly always passed around metafied (and
exceptions should be clearly documented, but I bet they aren't), but system
calls don't know anything about metafication.

> Obviously I can't use unmeta()
> twice in one function call though, can I call unmetafy() on the argv
> values or will something be sad then?

Yes, that should be fine.  argv is off the heap and commonly used as
workspace.

> Can the length grow when I unmetafy so I would need to alloc more space?

No, it will only ever remove Meta bytes (and xor the following one with
32).

> I also note the cap.c file doesn't unmeta(fy) its arguments so it
> probably also doesn't work, but I don't have cap stuff so can't test.

Yes, that's entirely possible; it probably hasn't been well tested with
new-fangled file names.

> Also, in unmeta() would putting a break; in the initial loop help? ie
>     for (t = file_name; *t; t++) {
> 	if (*t == Meta) {
> 	    meta = 1;
> +           break;
>         }
>     }

No, we need "t" to point to the end of the file name in this case for the
size check.

> My impression from looking at the code is that only metafy()ing can
> grow the string, so it should be safe to just unmetafy() the strings,
> assuming nothing breaks from me modifying the argv strings?

That's right.  The place to be wary about is if you later need to pass the
same strings to something else internally, but you can probably get away
without that.

-- 
Peter Stephenson <pws@csr.com>                  Software Engineer
CSR PLC, Churchill House, Cambridge Business Park, Cowley Road
Cambridge, CB4 0WZ, UK                          Tel: +44 (0)1223 692070

Re: [wip patch] new zsh/attr module

[Mikael Magnusson]

On Tue, 3 Mar 2009, Peter Stephenson wrote:

> On Thu, 26 Feb 2009 22:55:47 +0100 (CET)
> Mikael Magnusson <mikachu@gmail.com> wrote:
>> So I cobbled together this module to make three builtins,
>> zgetattr, zsetattr and zdelattr.
>>
>> #usage, *(e:fattr name:) or *(e:fattr name value:)
>> function fattr() {
>>    local val
>>    zgetattr $REPLY user.$1 val 2>/dev/null
>>    [[ -n "$val" && ( -z "$2" || "$val" =~ "$2" ) ]]
>> }
>
> This looks useful...
>
>> I'm not sure if I should mention it being copied from cap.c since pretty
>> much only the skeleton remains.
>
> I think not.

Removed

>> I guess I would have to write some documentation too.
>
> Yes.

I've written some basic entries for the builtins, it doesn't seem to show 
up properly in man zshmodules though, in the summary list at the top it 
doesn't appear, but it does appear below with the details.

>> The builtins should probably handle more than one file
>
> Right, then you'd need to use arrays to return values.  There's
> an argument for an option to put name/value pairs into associative arrays;
> that doesn't give you anything fundamentally new but it does prevent you
> having to loop over a file name and an attribute array at the same time
> which is potentially slow.

I've not done this yet.

>> and parse options in a better way.
>
> I think the optional parameter argument is OK; however, adding options is
> trivial since you're going through the standard builtin handling code.

It's trivial if you know how the standard builtin handling code works, 
less so otherwise :). I will look into it later. I think I want a -h
option for not dereferencing symlinks.

>> A builtin for listing attrs on a file
>> would be useful too (could at least be used for completion of the second
>> argument :) ).
>
> Yes, and in this case a missing parameter should just output to standard
> output.  I'm in two minds as to whether that would be better with zgetattr
> than defaulting to REPLY; it is more general since you can always specify
> REPLY if you want to use it (and with more than one file it would be
> "reply" anyway), and if the listing command does this it might be more
> consistent for the get command to do so.

I've changed zgetattr to printf the value if you don't specify a 
parameter, and added zlistattr, as well as a completer.

I didn't quite figure out how to best set an array parameter, so for now I 
still only handle one file, and the zlistattr function sets the whole 
string with nulls in the parameter, so you have to use ${(0)REPLY} to 
split it. I looked a little at bin_read since i know read can set an array 
parameter, but it is probably not very well suited as an easy to 
understand example since it does so many other things too.

>> Maybe the module should be called xattr instead of just attr?
>
> I'm not particularly bothered either way.  Too many x's and z's is a bit
> ugly.  However, there are multiple attribute systems and there's an
> argument for being specific.

I guess we can keep attr, that is what the package that contains getfattr 
and friends is called.

>> I also didn't bother checking what happens when the system doesn't
>> support xattrs or doesn't have the includes. I guess something similar to
>> what db_gdbm.mdd does is needed? I noticed just now that I was lazy and
>> used the ?: extension so that's something to fix too.
>
> Yes, you need to conditionalise linking by putting some configure
> (i.e. portable shell code) tests into link=`...` in the .mdd file.
> You should probably test both that the function and the header exist.
>
>> I think the AC_CHECK_LIB isn't exactly right either, it adds a second -lc
>> to $LIBS.
>
> You should just add the getxattr test to the AC_CHECK_FUNCS lib.

I think I got this right now, but I haven't tested the case where the 
stuff isn't found.

>> It seems these *argv and the ones below need to be wrapped in unmeta()
>> to work with utf-8 filenames/values.
>
> Yes, indeed:  internal values are nearly always passed around metafied (and
> exceptions should be clearly documented, but I bet they aren't), but system
> calls don't know anything about metafication.
>
>> Obviously I can't use unmeta()
>> twice in one function call though, can I call unmetafy() on the argv
>> values or will something be sad then?
>
> Yes, that should be fine.  argv is off the heap and commonly used as
> workspace.
>
>> Can the length grow when I unmetafy so I would need to alloc more space?
>
> No, it will only ever remove Meta bytes (and xor the following one with
> 32).
>
>> I also note the cap.c file doesn't unmeta(fy) its arguments so it
>> probably also doesn't work, but I don't have cap stuff so can't test.
>
> Yes, that's entirely possible; it probably hasn't been well tested with
> new-fangled file names.
>
>> My impression from looking at the code is that only metafy()ing can
>> grow the string, so it should be safe to just unmetafy() the strings,
>> assuming nothing breaks from me modifying the argv strings?
>
> That's right.  The place to be wary about is if you later need to pass the
> same strings to something else internally, but you can probably get away
> without that.

It looks like I have to re-metafy the string when using zwarnnam, I pass 
in the parameter that says the buffer is large enough, since it did 
contain the metafied version of the string some milliseconds earlier. That 
should work, right?

diff --git a/Completion/Zsh/Command/.distfiles b/Completion/Zsh/Command/.distfiles
index 0ef27ae..b1eb082 100644
--- a/Completion/Zsh/Command/.distfiles
+++ b/Completion/Zsh/Command/.distfiles
@@ -38,6 +38,7 @@ _unsetopt
  _vared
  _wait
  _which
+_zattr
  _zcompile
  _zed
  _zftp
diff --git a/Completion/Zsh/Command/_zattr b/Completion/Zsh/Command/_zattr
new file mode 100644
index 0000000..e3836f2
--- /dev/null
+++ b/Completion/Zsh/Command/_zattr
@@ -0,0 +1,34 @@
+#compdef zgetattr zsetattr zdelattr zlistattr
+
+local state line expl ret=1 REPLY
+local -a args privs
+
+case $service in
+zgetattr)
+_arguments \
+  '1:file:_files' \
+  '2:attribute:->attrs' \
+  '3:parameter'
+;;
+zsetattr)
+_arguments \
+  '1:file:_files' \
+  '2:attribute:->attrs' \
+  '3:value'
+;;
+zdelattr)
+_arguments \
+  '1:file:_files' \
+  '2:attribute:->attrs'
+;;
+zlistattr)
+_arguments \
+  '1:file:_files' \
+  '2:parameter'
+;;
+esac
+
+if [[ $state = attrs ]]; then
+  zlistattr $line[1] REPLY
+  _wanted attrs expl 'attribute' compadd ${(0)REPLY}
+fi
diff --git a/Doc/Makefile.in b/Doc/Makefile.in
index 7ca9dd2..2c07dc1 100644
--- a/Doc/Makefile.in
+++ b/Doc/Makefile.in
@@ -55,7 +55,7 @@ zshall.1
  YODLDOC = $(MAN) texi

  MODDOCSRC = \
-Zsh/mod_cap.yo Zsh/mod_clone.yo \
+Zsh/mod_attr.yo Zsh/mod_cap.yo Zsh/mod_clone.yo \
  Zsh/mod_compctl.yo Zsh/mod_complete.yo Zsh/mod_complist.yo \
  Zsh/mod_computil.yo Zsh/mod_curses.yo \
  Zsh/mod_datetime.yo Zsh/mod_deltochar.yo \
diff --git a/Doc/Zsh/.distfiles b/Doc/Zsh/.distfiles
index 4a2ea4f..9576487 100644
--- a/Doc/Zsh/.distfiles
+++ b/Doc/Zsh/.distfiles
@@ -5,7 +5,8 @@ DISTFILES_SRC='
      contrib.yo        exec.yo           expn.yo           filelist.yo
      files.yo          func.yo           grammar.yo        index.yo
      intro.yo          invoke.yo         jobs.yo           manmodmenu.yo
-    manual.yo         metafaq.yo        mod_cap.yo        mod_clone.yo
+    manual.yo         metafaq.yo        mod_attr.yo       mod_cap.yo
+    mod_clone.yo
      mod_compctl.yo    mod_complete.yo   mod_complist.yo   mod_computil.yo
      mod_curses.yo     mod_datetime.yo   mod_deltochar.yo  mod_example.yo
      mod_files.yo      mod_langinfo.yo   modlist.yo        mod_mapfile.yo
diff --git a/Doc/Zsh/mod_attr.yo b/Doc/Zsh/mod_attr.yo
new file mode 100644
index 0000000..ed444d0
--- /dev/null
+++ b/Doc/Zsh/mod_attr.yo
@@ -0,0 +1,34 @@
+COMMENT(!MOD!zsh/attr
+Builtins for manipulating extended attributes (xattr).
+!MOD!)
+The tt(zsh/attr) module is used for manipulating extended attributes.
+The builtins in this module are:
+
+startitem()
+findex(zgetattr)
+cindex(extended attributes, xattr, getting from files)
+item(tt(zgetattr) var(filename) var(attribute) [ var(parameter) ])(
+Get the extended attribute var(attribute) from the specified
+var(filename). If the optional argument var(parameter) is given, the
+attribute is set on that parameter instead of being printed to stdout.
+)
+findex(zsetattr)
+cindex(extended attributes, xattr, setting on files)
+item(tt(zsetattr) var(filename) var(attribute) var(value))(
+Set the extended attribute var(attribute) on the specified
+var(filename) to var(value).
+)
+findex(zdelattr)
+cindex(extended attributes, xattr, removing, deleting)
+item(tt(zdelattr) var(filename) var(attribute))(
+Remove the extended attribute var(attribute) from the specified
+var(filename).
+)
+findex(zlistattr)
+cindex(extended attributes, xattr, listing)
+item(tt(zlistattr) var(filename) [ var(parameter) ])(
+List the extended attributes currently set on the specified
+var(filename). If the optional argument var(parameter) is given, the
+list of attributes is set on that parameter instead of being printed to stdout.
+)
+enditem()
diff --git a/Src/Modules/.distfiles b/Src/Modules/.distfiles
index 40d3114..9231cec 100644
--- a/Src/Modules/.distfiles
+++ b/Src/Modules/.distfiles
@@ -2,6 +2,8 @@ DISTFILES_SRC='
  .cvsignore
  .distfiles
  .exrc
+attr.mdd
+attr.c
  cap.mdd
  cap.c
  clone.mdd
diff --git a/Src/Modules/attr.c b/Src/Modules/attr.c
new file mode 100644
index 0000000..01daf81
--- /dev/null
+++ b/Src/Modules/attr.c
@@ -0,0 +1,174 @@
+/*
+ * attr.c - extended attributes (xattr) manipulation
+ *
+ * This file is part of zsh, the Z shell.
+ *
+ * Copyright (c) 2009 Mikael Magnusson
+ * All rights reserved.
+ *
+ * Permission is hereby granted, without written agreement and without
+ * license or royalty fees, to use, copy, modify, and distribute this
+ * software and to distribute modified versions of this software for any
+ * purpose, provided that the above copyright notice and the following
+ * two paragraphs appear in all copies of this software.
+ *
+ * In no event shall Mikael Magnusson or the Zsh Development Group be liable
+ * to any party for direct, indirect, special, incidental, or consequential
+ * damages arising out of the use of this software and its documentation,
+ * even if Andrew Main and the Zsh Development Group have been advised of
+ * the possibility of such damage.
+ *
+ * Mikael Magnusson and the Zsh Development Group specifically disclaim any
+ * warranties, including, but not limited to, the implied warranties of
+ * merchantability and fitness for a particular purpose.  The software
+ * provided hereunder is on an "as is" basis, and Mikael Magnusson and the
+ * Zsh Development Group have no obligation to provide maintenance,
+ * support, updates, enhancements, or modifications.
+ *
+ */
+
+#include "attr.mdh"
+#include "attr.pro"
+
+#include <sys/types.h>
+#include <sys/xattr.h>
+
+static int
+bin_getattr(char *nam, char **argv, UNUSED(Options ops), UNUSED(int func))
+{
+    int ret = 0;
+    int len;
+    char value[256];
+
+    unmetafy(*argv, NULL);
+    unmetafy(*(argv+1), NULL);
+    if (listxattr(*argv, NULL, 0) > 0) {
+        if (0 < (len = getxattr(*argv, *(argv+1), value, 255))) {
+            if (len < 256) {
+                value[len] = '\0';
+                if (*(argv+2))
+                    setsparam(*(argv+2), metafy(value, len, META_DUP));
+                else
+                    printf("%s\n", value);
+            }
+        } else {
+            zwarnnam(nam, "%s: %e", metafy(*argv, -1, META_NOALLOC), errno);
+            ret = 1;
+        }
+    }
+    return ret;
+}
+
+static int
+bin_setattr(char *nam, char **argv, UNUSED(Options ops), UNUSED(int func))
+{
+    int ret = 0;
+ 
+    unmetafy(*argv, NULL);
+    unmetafy(*(argv+1), NULL);
+    unmetafy(*(argv+2), NULL);
+    if (setxattr(*argv, *(argv+1), *(argv+2), strlen(*(argv+2)), 0)) {
+        zwarnnam(nam, "%s: %e", metafy(*argv, -1, META_NOALLOC), errno);
+        ret = 1;
+    }
+    return ret;
+}
+
+static int
+bin_delattr(char *nam, char **argv, UNUSED(Options ops), UNUSED(int func))
+{
+    int ret = 0;
+ 
+    unmetafy(*argv, NULL);
+    unmetafy(*(argv+1), NULL);
+    if (removexattr(*argv, *(argv+1))) {
+        zwarnnam(nam, "%s: %e", metafy(*argv, -1, META_NOALLOC), errno);
+        ret = 1;
+    }
+    return ret;
+}
+ 
+static int
+bin_listattr(char *nam, char **argv, UNUSED(Options ops), UNUSED(int func))
+{
+    int ret = 0;
+    int len, i = 1;
+    char value[256];
+
+    unmetafy(*argv, NULL);
+    if (0 < (len = listxattr(*argv, value, 256))) {
+        if (len < 256) {
+            char *p = value;
+            if (*(argv+1))
+                setsparam(*(argv+1), metafy(value, len, META_DUP));
+            else while (p < &value[len]) {
+                printf("%s\n", p);
+                p += strlen(p) + 1;
+            }
+        }
+    } else {
+        zwarnnam(nam, "%s: %e", metafy(*argv, -1, META_NOALLOC), errno);
+        ret = 1;
+    }
+    return ret;
+}
+
+/* module paraphernalia */
+
+static struct builtin bintab[] = {
+    BUILTIN("zgetattr", 0, bin_getattr, 2, 3, 0, NULL, NULL),
+    BUILTIN("zsetattr", 0, bin_setattr, 3, 3, 0, NULL, NULL),
+    BUILTIN("zdelattr", 0, bin_delattr, 2, 2, 0, NULL, NULL),
+    BUILTIN("zlistattr", 0, bin_listattr, 1, 2, 0, NULL, NULL),
+};
+
+static struct features module_features = {
+    bintab, sizeof(bintab)/sizeof(*bintab),
+    NULL, 0,
+    NULL, 0,
+    NULL, 0,
+    0
+};
+
+/**/
+int
+setup_(UNUSED(Module m))
+{
+    return 0;
+}
+
+/**/
+int
+features_(Module m, char ***features)
+{
+    *features = featuresarray(m, &module_features);
+    return 0;
+}
+
+/**/
+int
+enables_(Module m, int **enables)
+{
+    return handlefeatures(m, &module_features, enables);
+}
+
+/**/
+int
+boot_(UNUSED(Module m))
+{
+    return 0;
+}
+
+/**/
+int
+cleanup_(Module m)
+{
+    return setfeatureenables(m, &module_features, NULL);
+}
+
+/**/
+int
+finish_(UNUSED(Module m))
+{
+    return 0;
+}
diff --git a/Src/Modules/attr.mdd b/Src/Modules/attr.mdd
new file mode 100644
index 0000000..52f3e24
--- /dev/null
+++ b/Src/Modules/attr.mdd
@@ -0,0 +1,12 @@
+name=zsh/attr
+link='if test "x$ac_cv_func_getxattr" = xyes && test "x$ac_cv_header_sys_xattr_h" = xyes; then
+  echo dynamic
+else
+  echo no
+fi
+'
+load=no
+
+autofeatures="b:zgetattr b:zsetattr b:zdelattr"
+
+objects="attr.o"
diff --git a/configure.ac b/configure.ac
index d67f203..1360865 100644
--- a/configure.ac
+++ b/configure.ac
@@ -858,6 +858,8 @@ if test x$gdbm != xno; then
    AC_CHECK_LIB(gdbm, gdbm_open)
  fi

+AC_CHECK_HEADERS(sys/xattr.h)
+
  dnl --------------
  dnl CHECK TYPEDEFS
  dnl --------------
@@ -1155,7 +1157,7 @@ AC_CHECK_FUNCS(strftime strptime mktime timelocal \
  	       grantpt unlockpt ptsname \
  	       htons ntohs \
  	       regcomp regexec regerror regfree \
-	       gdbm_open)
+	       gdbm_open getxattr)
  AC_FUNC_STRCOLL

  if test x$enable_cap = xyes; then

--
Mikael Magnusson

Re: [wip patch] new zsh/attr module

[Peter Stephenson]

On Tue, 3 Mar 2009 15:43:29 +0100 (CET)
Mikael Magnusson <mikachu@gmail.com> wrote:
> I've written some basic entries for the builtins, it doesn't seem to show 
> up properly in man zshmodules though, in the summary list at the top it 
> doesn't appear, but it does appear below with the details.

It might be just a dependency problem; it all seems to be there in my case.

> It's trivial if you know how the standard builtin handling code works, 
> less so otherwise :). I will look into it later. I think I want a -h
> option for not dereferencing symlinks.

You need a string containing "h" where the builtin is declared and to test
OPT_ISSET(ops,'h') in the function for the builtin.

> I didn't quite figure out how to best set an array parameter, so for now I 
> still only handle one file, and the zlistattr function sets the whole 
> string with nulls in the parameter, so you have to use ${(0)REPLY} to 
> split it. I looked a little at bin_read since i know read can set an array 
> parameter, but it is probably not very well suited as an easy to 
> understand example since it does so many other things too.

The use of setaparam() in stat.c is probably a good example, it's doing
something very similar, and there are some hash examples down there, too.
Remember all the bits come from permanently allocated memory.

> I think I got this right now, but I haven't tested the case where the 
> stuff isn't found.

It looks OK.  I'll see if I can find a Solaris machine left over to try it
on when I've committed the patch at the bottom.

> It looks like I have to re-metafy the string when using zwarnnam, I pass 
> in the parameter that says the buffer is large enough, since it did 
> contain the metafied version of the string some milliseconds earlier. That 
> should work, right?

It's better to pass in the right length in case of embedded NULs---there
shouldn't be any since the filesystem calls wouldn't handle them, but it's
more consistent.  That's an easy change.

Something strange happened with some of the whitespace in the patch so I
applied various bits by hand.

There are various minor tweaks below: expand the filename in completion in
case it's got a ~ in it; reformat the .distfiles (I've been gradually doing
this to all of them since one file per line is much easier to maintain);
handle lengths when metafying; don't report errors unless system calls
returned a negative value; remove an unused variable.

===================================================================
RCS file: /cvsroot/zsh/zsh/Completion/Zsh/Command/_zattr,v
retrieving revision 1.1
diff -u -r1.1 _zattr
--- Completion/Zsh/Command/_zattr	3 Mar 2009 15:04:27 -0000	1.1
+++ Completion/Zsh/Command/_zattr	3 Mar 2009 16:24:48 -0000
@@ -29,6 +29,6 @@
 esac
 
 if [[ $state = attrs ]]; then
-  zlistattr $line[1] REPLY
+  zlistattr $~line[1] REPLY
   _wanted attrs expl 'attribute' compadd ${(0)REPLY}
 fi
Index: Doc/Zsh/.distfiles
===================================================================
RCS file: /cvsroot/zsh/zsh/Doc/Zsh/.distfiles,v
retrieving revision 1.19
diff -u -r1.19 .distfiles
--- Doc/Zsh/.distfiles	3 Mar 2009 15:04:28 -0000	1.19
+++ Doc/Zsh/.distfiles	3 Mar 2009 16:24:48 -0000
@@ -1,21 +1,72 @@
 DISTFILES_SRC='
-    .cvsignore .distfiles
-    arith.yo          builtins.yo       calsys.yo         compat.yo
-    compctl.yo        compsys.yo        compwid.yo        cond.yo
-    contrib.yo        exec.yo           expn.yo           filelist.yo
-    files.yo          func.yo           grammar.yo        index.yo
-    intro.yo          invoke.yo         jobs.yo           manmodmenu.yo
-    manual.yo         metafaq.yo        mod_attr.yo       mod_cap.yo
-    mod_clone.yo
-    mod_compctl.yo    mod_complete.yo   mod_complist.yo   mod_computil.yo
-    mod_curses.yo     mod_datetime.yo   mod_deltochar.yo  mod_example.yo
-    mod_files.yo      mod_langinfo.yo   modlist.yo        mod_mapfile.yo
-    mod_mathfunc.yo   modmenu.yo        mod_newuser.yo    mod_parameter.yo
-    mod_pcre.yo       mod_regex.yo      mod_sched.yo      mod_socket.yo
-    mod_stat.yo       mod_system.yo     mod_tcp.yo        mod_termcap.yo
-    mod_terminfo.yo   modules.yo        mod_zftp.yo       mod_zleparameter.yo
-    mod_zle.yo        mod_zprof.yo      mod_zpty.yo       mod_zselect.yo
-    mod_zutil.yo      options.yo        params.yo         prompt.yo
-    redirect.yo       restricted.yo     roadmap.yo        seealso.yo
-    tcpsys.yo         zftpsys.yo        zle.yo
+.cvsignore
+.distfiles
+arith.yo
+builtins.yo
+calsys.yo
+compat.yo
+compctl.yo
+compsys.yo
+compwid.yo
+cond.yo
+contrib.yo
+exec.yo
+expn.yo
+filelist.yo
+files.yo
+func.yo
+grammar.yo
+index.yo
+intro.yo
+invoke.yo
+jobs.yo
+manmodmenu.yo
+manual.yo
+metafaq.yo
+mod_attr.yo
+mod_cap.yo
+mod_clone.yo
+mod_compctl.yo
+mod_complete.yo
+mod_complist.yo
+mod_computil.yo
+mod_curses.yo
+mod_datetime.yo
+mod_deltochar.yo
+mod_example.yo
+mod_files.yo
+mod_langinfo.yo
+modlist.yo
+mod_mapfile.yo
+mod_mathfunc.yo
+modmenu.yo
+mod_newuser.yo
+mod_parameter.yo
+mod_pcre.yo
+mod_regex.yo
+mod_sched.yo
+mod_socket.yo
+mod_stat.yo
+mod_system.yo
+mod_tcp.yo
+mod_termcap.yo
+mod_terminfo.yo
+modules.yo
+mod_zftp.yo
+mod_zleparameter.yo
+mod_zle.yo
+mod_zprof.yo
+mod_zpty.yo
+mod_zselect.yo
+mod_zutil.yo
+options.yo
+params.yo
+prompt.yo
+redirect.yo
+restricted.yo
+roadmap.yo
+seealso.yo
+tcpsys.yo
+zftpsys.yo
+zle.yo
 '
Index: Src/Modules/attr.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/Modules/attr.c,v
retrieving revision 1.1
diff -u -r1.1 attr.c
--- Src/Modules/attr.c	3 Mar 2009 15:04:28 -0000	1.1
+++ Src/Modules/attr.c	3 Mar 2009 16:24:48 -0000
@@ -37,10 +37,10 @@
 bin_getattr(char *nam, char **argv, UNUSED(Options ops), UNUSED(int func))
 {
     int ret = 0;
-    int len;
+    int len, slen;
     char value[256];
 
-    unmetafy(*argv, NULL);
+    unmetafy(*argv, &slen);
     unmetafy(*(argv+1), NULL);
     if (listxattr(*argv, NULL, 0) > 0) {
         if (0 < (len = getxattr(*argv, *(argv+1), value, 255))) {
@@ -51,8 +51,8 @@
                 else
                     printf("%s\n", value);
             }
-        } else {
-            zwarnnam(nam, "%s: %e", metafy(*argv, -1, META_NOALLOC), errno);
+        } else if (len < 0) {
+            zwarnnam(nam, "%s: %e", metafy(*argv, slen, META_NOALLOC), errno);
             ret = 1;
         }
     }
@@ -62,13 +62,13 @@
 static int
 bin_setattr(char *nam, char **argv, UNUSED(Options ops), UNUSED(int func))
 {
-    int ret = 0;
- 
-    unmetafy(*argv, NULL);
+    int ret = 0, slen;
+
+    unmetafy(*argv, &slen);
     unmetafy(*(argv+1), NULL);
     unmetafy(*(argv+2), NULL);
     if (setxattr(*argv, *(argv+1), *(argv+2), strlen(*(argv+2)), 0)) {
-        zwarnnam(nam, "%s: %e", metafy(*argv, -1, META_NOALLOC), errno);
+        zwarnnam(nam, "%s: %e", metafy(*argv, slen, META_NOALLOC), errno);
         ret = 1;
     }
     return ret;
@@ -77,25 +77,25 @@
 static int
 bin_delattr(char *nam, char **argv, UNUSED(Options ops), UNUSED(int func))
 {
-    int ret = 0;
- 
-    unmetafy(*argv, NULL);
+    int ret = 0, slen;
+
+    unmetafy(*argv, &slen);
     unmetafy(*(argv+1), NULL);
     if (removexattr(*argv, *(argv+1))) {
-        zwarnnam(nam, "%s: %e", metafy(*argv, -1, META_NOALLOC), errno);
+        zwarnnam(nam, "%s: %e", metafy(*argv, slen, META_NOALLOC), errno);
         ret = 1;
     }
     return ret;
 }
- 
+
 static int
 bin_listattr(char *nam, char **argv, UNUSED(Options ops), UNUSED(int func))
 {
     int ret = 0;
-    int len, i = 1;
+    int len, slen;
     char value[256];
 
-    unmetafy(*argv, NULL);
+    unmetafy(*argv, &slen);
     if (0 < (len = listxattr(*argv, value, 256))) {
         if (len < 256) {
             char *p = value;
@@ -106,8 +106,8 @@
                 p += strlen(p) + 1;
             }
         }
-    } else {
-        zwarnnam(nam, "%s: %e", metafy(*argv, -1, META_NOALLOC), errno);
+    } else if (len < 0) {
+        zwarnnam(nam, "%s: %e", metafy(*argv, slen, META_NOALLOC), errno);
         ret = 1;
     }
     return ret;



-- 
Peter Stephenson <pws@csr.com>                  Software Engineer
CSR PLC, Churchill House, Cambridge Business Park, Cowley Road
Cambridge, CB4 0WZ, UK                          Tel: +44 (0)1223 692070

Re: [wip patch] new zsh/attr module

[Mikael Magnusson]

2009/3/3 Peter Stephenson <pws@csr.com>:
> On Tue, 3 Mar 2009 15:43:29 +0100 (CET)
> Mikael Magnusson <mikachu@gmail.com> wrote:
>> I've written some basic entries for the builtins, it doesn't seem to show
>> up properly in man zshmodules though, in the summary list at the top it
>> doesn't appear, but it does appear below with the details.
>
> It might be just a dependency problem; it all seems to be there in my case.

Okay, I'll see if it works after some more make cleaning.

>> It's trivial if you know how the standard builtin handling code works,
>> less so otherwise :). I will look into it later. I think I want a -h
>> option for not dereferencing symlinks.
>
> You need a string containing "h" where the builtin is declared and to test
> OPT_ISSET(ops,'h') in the function for the builtin.
>
>> I didn't quite figure out how to best set an array parameter, so for now I
>> still only handle one file, and the zlistattr function sets the whole
>> string with nulls in the parameter, so you have to use ${(0)REPLY} to
>> split it. I looked a little at bin_read since i know read can set an array
>> parameter, but it is probably not very well suited as an easy to
>> understand example since it does so many other things too.
>
> The use of setaparam() in stat.c is probably a good example, it's doing
> something very similar, and there are some hash examples down there, too.
> Remember all the bits come from permanently allocated memory.
>
>> I think I got this right now, but I haven't tested the case where the
>> stuff isn't found.
>
> It looks OK.  I'll see if I can find a Solaris machine left over to try it
> on when I've committed the patch at the bottom.
>
>> It looks like I have to re-metafy the string when using zwarnnam, I pass
>> in the parameter that says the buffer is large enough, since it did
>> contain the metafied version of the string some milliseconds earlier. That
>> should work, right?
>
> It's better to pass in the right length in case of embedded NULs---there
> shouldn't be any since the filesystem calls wouldn't handle them, but it's
> more consistent.  That's an easy change.
>
> Something strange happened with some of the whitespace in the patch so I
> applied various bits by hand.

Ah, I didn't really expect anyone to actually apply it yet but I did
send it through alpine so it's odd that the whitespace was corrupted.

> There are various minor tweaks below: expand the filename in completion in
> case it's got a ~ in it

The completer also needs to check if the file exists before calling
zgetattr, or simply 2> /dev/null I guess.

> reformat the .distfiles (I've been gradually doing
> this to all of them since one file per line is much easier to maintain);
> handle lengths when metafying; don't report errors unless system calls
> returned a negative value; remove an unused variable.

I know the codestyle is a bit questionable but I already check 0 < len
in the first if, so the else will be all negative lengths already.

Another thing that needs to be done (for small values of need) is to
allocate larger buffers in case the actual attributes take up more
space than 256 bytes (or whatever value is a good default first try).

I'll look at the other stuff a little later, thanks for your help so far.

-- 
Mikael Magnusson

Re: [wip patch] new zsh/attr module

[Peter Stephenson]

Mikael Magnusson wrote:
> > don't report errors unless system calls returned a negative value
> 
> I know the codestyle is a bit questionable but I already check 0 < len
> in the first if, so the else will be all negative lengths already.

... or zero, which was the problem.  It was printing an error message
with errno 0 if the list returned empty.  It now does the standard
Unix-style thing of printing nothing but returning status zero.

-- 
Peter Stephenson <pws@csr.com>                  Software Engineer
CSR PLC, Churchill House, Cambridge Business Park, Cowley Road
Cambridge, CB4 0WZ, UK                          Tel: +44 (0)1223 692070

Re: [wip patch] new zsh/attr module

[Mikael Magnusson]

2009/3/3 Peter Stephenson <pws@csr.com>:
> Mikael Magnusson wrote:
>> > don't report errors unless system calls returned a negative value
>>
>> I know the codestyle is a bit questionable but I already check 0 < len
>> in the first if, so the else will be all negative lengths already.
>
> ... or zero, which was the problem.  It was printing an error message
> with errno 0 if the list returned empty.  It now does the standard
> Unix-style thing of printing nothing but returning status zero.

Uh, right. I must have left my head somewhere else :).


-- 
Mikael Magnusson

Re: [wip patch] new zsh/attr module

[Mikael Magnusson]

Or much later, as it turns out. I fixed most of it I think. I haven't
tried the OSX-style interface, but I checked their docs so I think it's
right.

attr: add -h option to operate on symlinks without dereferencing
  I don't have any filesystem that supports xattrs on symlinks, so this
  is also untested.
attr: Make zlistattr return an array, zdelattr take several attrs
attr: dynamically allocate memory for attributes
attr: Use descriptive variables for argv and allow setting values with embedded nuls

[PATCH 1/4] attr: add -h option to operate on symlinks without dereferencing

[Mikael Magnusson]

Since the OSX-style interface doesn't have the l* family, make wrapper
functions that handle the boring details.
---
  Src/Modules/attr.c |  111 ++++++++++++++++++++++++++++++++++++---------------
  1 files changed, 78 insertions(+), 33 deletions(-)

diff --git a/Src/Modules/attr.c b/Src/Modules/attr.c
index ec3b1e4..cbf3d92 100644
--- a/Src/Modules/attr.c
+++ b/Src/Modules/attr.c
@@ -33,25 +33,79 @@
  #include <sys/types.h>
  #include <sys/xattr.h>

+static ssize_t
+xgetxattr(const char *path, const char *name, void *value, size_t size, int symlink)
+{
+#ifdef XATTR_EXTRA_ARGS
+    return getxattr(path, name, value, size, 0, symlink ? XATTR_NOFOLLOW: 0);
+#else
+    switch (symlink) {
+    case 0:
+        return getxattr(path, name, value, size);
+    case 1:
+        return lgetxattr(path, name, value, size);
+    }
+#endif
+}
+
+static ssize_t
+xlistxattr(const char *path, char *list, size_t size, int symlink)
+{
+#ifdef XATTR_EXTRA_ARGS
+    return listxattr(path, list, size, symlink ? XATTR_NOFOLLOW : 0);
+#else
+    switch (symlink) {
+    case 0:
+        return listxattr(path, list, size);
+    case 1:
+        return llistxattr(path, list, size);
+    }
+#endif
+}
+
+static int
+xsetxattr(const char *path, const char *name, const void *value,
+          size_t size, int flags, int symlink)
+{
+#ifdef XATTR_EXTRA_ARGS
+    return setxattr(path, name, value, size, 0, flags | symlink ? XATTR_NOFOLLOW : 0);
+#else
+    switch (symlink) {
+    case 0:
+        return setxattr(path, name, value, size, flags);
+    case 1:
+        return lsetxattr(path, name, value, size, flags);
+    }
+#endif
+}
+
  static int
-bin_getattr(char *nam, char **argv, UNUSED(Options ops), UNUSED(int func))
+xremovexattr(const char *path, const char *name, int symlink)
+{
+#ifdef XATTR_EXTRA_ARGS
+    return removexattr(path, name, symlink ? XATTR_NOFOLLOW : 0);
+#else
+    switch (symlink) {
+    case 0:
+        return removexattr(path, name);
+    case 1:
+        return lremovexattr(path, name);
+    }
+#endif
+}
+
+static int
+bin_getattr(char *nam, char **argv, Options ops, UNUSED(int func))
  {
      int ret = 0;
      int len, slen;
      char value[256];
+    int symlink = OPT_ISSET(ops, 'h');

      unmetafy(*argv, &slen);
      unmetafy(*(argv+1), NULL);
-    if (listxattr(*argv, NULL, 0
-#ifdef XATTR_EXTRA_ARGS
-		  , 0
-#endif
-		  ) > 0) {
-        if (0 < (len = getxattr(*argv, *(argv+1), value, 255
-#ifdef XATTR_EXTRA_ARGS
-				, 0, 0
-#endif
-				))) {
+    if (xlistxattr(*argv, NULL, 0, symlink) > 0) {
+        if (0 < (len = xgetxattr(*argv, *(argv+1), value, 255, symlink))) {
              if (len < 256) {
                  value[len] = '\0';
                  if (*(argv+2))
@@ -68,18 +122,15 @@ bin_getattr(char *nam, char **argv, UNUSED(Options ops), UNUSED(int func))
  }

  static int
-bin_setattr(char *nam, char **argv, UNUSED(Options ops), UNUSED(int func))
+bin_setattr(char *nam, char **argv, Options ops, UNUSED(int func))
  {
      int ret = 0, slen;
+    int symlink = OPT_ISSET(ops, 'h');

      unmetafy(*argv, &slen);
      unmetafy(*(argv+1), NULL);
      unmetafy(*(argv+2), NULL);
-    if (setxattr(*argv, *(argv+1), *(argv+2), strlen(*(argv+2)), 0
-#ifdef XATTR_EXTRA_ARGS
-						     , 0
-#endif
-		 )) {
+    if (xsetxattr(*argv, *(argv+1), *(argv+2), strlen(*(argv+2)), 0, symlink)) {
          zwarnnam(nam, "%s: %e", metafy(*argv, slen, META_NOALLOC), errno);
          ret = 1;
      }
@@ -87,17 +138,14 @@ bin_setattr(char *nam, char **argv, UNUSED(Options ops), UNUSED(int func))
  }

  static int
-bin_delattr(char *nam, char **argv, UNUSED(Options ops), UNUSED(int func))
+bin_delattr(char *nam, char **argv, Options ops, UNUSED(int func))
  {
      int ret = 0, slen;
+    int symlink = OPT_ISSET(ops, 'h');

      unmetafy(*argv, &slen);
      unmetafy(*(argv+1), NULL);
-    if (removexattr(*argv, *(argv+1)
-#ifdef XATTR_EXTRA_ARGS
-		    , 0
-#endif
-		    )) {
+    if (xremovexattr(*argv, *(argv+1), symlink)) {
          zwarnnam(nam, "%s: %e", metafy(*argv, slen, META_NOALLOC), errno);
          ret = 1;
      }
@@ -105,18 +153,15 @@ bin_delattr(char *nam, char **argv, UNUSED(Options ops), UNUSED(int func))
  }

  static int
-bin_listattr(char *nam, char **argv, UNUSED(Options ops), UNUSED(int func))
+bin_listattr(char *nam, char **argv, Options ops, UNUSED(int func))
  {
      int ret = 0;
      int len, slen;
      char value[256];
+    int symlink = OPT_ISSET(ops, 'h');

      unmetafy(*argv, &slen);
-    if (0 < (len = listxattr(*argv, value, 256
-#ifdef XATTR_EXTRA_ARGS
-		  , 0
-#endif
-			     ))) {
+    if (0 < (len = xlistxattr(*argv, value, 256, symlink))) {
          if (len < 256) {
              char *p = value;
              if (*(argv+1))
@@ -136,10 +181,10 @@ bin_listattr(char *nam, char **argv, UNUSED(Options ops), UNUSED(int func))
  /* module paraphernalia */

  static struct builtin bintab[] = {
-    BUILTIN("zgetattr", 0, bin_getattr, 2, 3, 0, NULL, NULL),
-    BUILTIN("zsetattr", 0, bin_setattr, 3, 3, 0, NULL, NULL),
-    BUILTIN("zdelattr", 0, bin_delattr, 2, 2, 0, NULL, NULL),
-    BUILTIN("zlistattr", 0, bin_listattr, 1, 2, 0, NULL, NULL),
+    BUILTIN("zgetattr", 0, bin_getattr, 2, 3, 0, "h", NULL),
+    BUILTIN("zsetattr", 0, bin_setattr, 3, 3, 0, "h", NULL),
+    BUILTIN("zdelattr", 0, bin_delattr, 2, 2, 0, "h", NULL),
+    BUILTIN("zlistattr", 0, bin_listattr, 1, 2, 0, "h", NULL),
  };

  static struct features module_features = {
-- 
1.6.5

[PATCH 2/4] attr: Make zlistattr return an array, zdelattr take several attrs

[Mikael Magnusson]

---
  Completion/Zsh/Command/_zattr |    2 +-
  Src/Modules/attr.c            |   39 ++++++++++++++++++++++++++++++---------
  2 files changed, 31 insertions(+), 10 deletions(-)

diff --git a/Completion/Zsh/Command/_zattr b/Completion/Zsh/Command/_zattr
index 55804db..a6ae806 100644
--- a/Completion/Zsh/Command/_zattr
+++ b/Completion/Zsh/Command/_zattr
@@ -31,6 +31,6 @@ esac
  if [[ $state = attrs ]]; then
    if [[ -f $~line[1] ]]; then
      zlistattr $~line[1] REPLY
-    _wanted attrs expl 'attribute' compadd ${(0)REPLY}
+    _wanted attrs expl 'attribute' compadd $REPLY
    fi
  fi
diff --git a/Src/Modules/attr.c b/Src/Modules/attr.c
index cbf3d92..1a9c323 100644
--- a/Src/Modules/attr.c
+++ b/Src/Modules/attr.c
@@ -142,12 +142,16 @@ bin_delattr(char *nam, char **argv, Options ops, UNUSED(int func))
  {
      int ret = 0, slen;
      int symlink = OPT_ISSET(ops, 'h');
+    char *file = *argv;

-    unmetafy(*argv, &slen);
-    unmetafy(*(argv+1), NULL);
-    if (xremovexattr(*argv, *(argv+1), symlink)) {
-        zwarnnam(nam, "%s: %e", metafy(*argv, slen, META_NOALLOC), errno);
-        ret = 1;
+    unmetafy(file, &slen);
+    while (*++argv) {
+        unmetafy(*argv, NULL);
+        if (xremovexattr(file, *argv, symlink)) {
+            zwarnnam(nam, "%s: %e", metafy(file, slen, META_NOALLOC), errno);
+            ret = 1;
+            break;
+        }
      }
      return ret;
  }
@@ -164,9 +168,26 @@ bin_listattr(char *nam, char **argv, Options ops, UNUSED(int func))
      if (0 < (len = xlistxattr(*argv, value, 256, symlink))) {
          if (len < 256) {
              char *p = value;
-            if (*(argv+1))
-                setsparam(*(argv+1), metafy(value, len, META_DUP));
-            else while (p < &value[len]) {
+            if (*(argv+1)) {
+                if (strlen(value) + 1 == len)
+                    setsparam(*(argv+1), metafy(value, len-1, META_DUP));
+                else {
+                    int arrlen = 0;
+                    char **array = NULL, **arrptr = NULL;
+
+                    while (p < &value[len]) {
+                        arrlen++;
+                        p += strlen(p) + 1;
+                    }
+                    arrptr = array = (char **)zshcalloc((arrlen+1) * sizeof(char *));
+                    p = value;
+                    while (p < &value[len]) {
+                        *arrptr++ = metafy(p, -1, META_DUP);
+                        p += strlen(p) + 1;
+                    }
+                    setaparam(*(argv+1), array);
+                }
+            } else while (p < &value[len]) {
                  printf("%s\n", p);
                  p += strlen(p) + 1;
              }
@@ -183,7 +204,7 @@ bin_listattr(char *nam, char **argv, Options ops, UNUSED(int func))
  static struct builtin bintab[] = {
      BUILTIN("zgetattr", 0, bin_getattr, 2, 3, 0, "h", NULL),
      BUILTIN("zsetattr", 0, bin_setattr, 3, 3, 0, "h", NULL),
-    BUILTIN("zdelattr", 0, bin_delattr, 2, 2, 0, "h", NULL),
+    BUILTIN("zdelattr", 0, bin_delattr, 2, -1, 0, "h", NULL),
      BUILTIN("zlistattr", 0, bin_listattr, 1, 2, 0, "h", NULL),
  };

-- 
1.6.5

[PATCH 3/4] attr: dynamically allocate memory for attributes

[Mikael Magnusson]

If there is a race condition and the attribute grows between the two
calls, we return an error instead of trying again.
---
  Doc/Zsh/mod_attr.yo |    5 ++++
  Src/Modules/attr.c  |   63 ++++++++++++++++++++++++++++++++++-----------------
  2 files changed, 47 insertions(+), 21 deletions(-)

diff --git a/Doc/Zsh/mod_attr.yo b/Doc/Zsh/mod_attr.yo
index ed444d0..517bb63 100644
--- a/Doc/Zsh/mod_attr.yo
+++ b/Doc/Zsh/mod_attr.yo
@@ -32,3 +32,8 @@ var(filename). If the optional argument var(parameter) is given, the
  list of attributes is set on that parameter instead of being printed to stdout.
  )
  enditem()
+
+tt(zgetattr) and tt(zlistattr) allocate memory dynamically. If the attribute or
+list of attributes grows between the allocation and the call to get them, they
+return 2. On all other errors, 1 is returned. This way, the calling function can
+check for this case and try again for as long as they want.
diff --git a/Src/Modules/attr.c b/Src/Modules/attr.c
index 1a9c323..bb30ebb 100644
--- a/Src/Modules/attr.c
+++ b/Src/Modules/attr.c
@@ -98,26 +98,37 @@ static int
  bin_getattr(char *nam, char **argv, Options ops, UNUSED(int func))
  {
      int ret = 0;
-    int len, slen;
-    char value[256];
+    int list_len, val_len, attr_len, slen;
+    char *value;
      int symlink = OPT_ISSET(ops, 'h');

      unmetafy(*argv, &slen);
      unmetafy(*(argv+1), NULL);
-    if (xlistxattr(*argv, NULL, 0, symlink) > 0) {
-        if (0 < (len = xgetxattr(*argv, *(argv+1), value, 255, symlink))) {
-            if (len < 256) {
-                value[len] = '\0';
+    list_len = xlistxattr(*argv, NULL, 0, symlink);
+    if (list_len > 0) {
+        val_len = xgetxattr(*argv, *(argv+1), NULL, 0, symlink);
+        if (val_len == 0) {
+            if (*(argv+2))
+                unsetparam(*(argv+2));
+            return 0;
+        }
+        if (val_len > 0) {
+            value = (char *)zalloc(val_len+1);
+            attr_len = xgetxattr(*argv, *(argv+1), value, val_len, symlink);
+            if (attr_len > 0 && attr_len <= val_len) {
+                value[attr_len] = '\0';
                  if (*(argv+2))
-                    setsparam(*(argv+2), metafy(value, len, META_DUP));
+                    setsparam(*(argv+2), metafy(value, attr_len, META_DUP));
                  else
                      printf("%s\n", value);
              }
-        } else if (len < 0) {
-            zwarnnam(nam, "%s: %e", metafy(*argv, slen, META_NOALLOC), errno);
-            ret = 1;
+            zfree(value, val_len+1);
          }
      }
+    if (list_len < 0 || val_len < 0 || attr_len < 0)  {
+        zwarnnam(nam, "%s: %e", metafy(*argv, slen, META_NOALLOC), errno);
+        ret = 1 + (attr_len > val_len);
+    }
      return ret;
  }

@@ -160,41 +171,51 @@ static int
  bin_listattr(char *nam, char **argv, Options ops, UNUSED(int func))
  {
      int ret = 0;
-    int len, slen;
-    char value[256];
+    int val_len, list_len, slen;
+    char *value;
      int symlink = OPT_ISSET(ops, 'h');

      unmetafy(*argv, &slen);
-    if (0 < (len = xlistxattr(*argv, value, 256, symlink))) {
-        if (len < 256) {
+    val_len = xlistxattr(*argv, NULL, 0, symlink);
+    if (val_len == 0) {
+        if (*(argv+1))
+            unsetparam(*(argv+1));
+        return 0;
+    }
+    if (val_len > 0) {
+        value = (char *)zalloc(val_len+1);
+        list_len = xlistxattr(*argv, value, val_len, symlink);
+        if (list_len > 0 && list_len <= val_len) {
              char *p = value;
              if (*(argv+1)) {
-                if (strlen(value) + 1 == len)
-                    setsparam(*(argv+1), metafy(value, len-1, META_DUP));
+                if (strlen(value) + 1 == list_len)
+                    setsparam(*(argv+1), metafy(value, list_len-1, META_DUP));
                  else {
                      int arrlen = 0;
                      char **array = NULL, **arrptr = NULL;

-                    while (p < &value[len]) {
+                    while (p < &value[list_len]) {
                          arrlen++;
                          p += strlen(p) + 1;
                      }
                      arrptr = array = (char **)zshcalloc((arrlen+1) * sizeof(char *));
                      p = value;
-                    while (p < &value[len]) {
+                    while (p < &value[list_len]) {
                          *arrptr++ = metafy(p, -1, META_DUP);
                          p += strlen(p) + 1;
                      }
                      setaparam(*(argv+1), array);
                  }
-            } else while (p < &value[len]) {
+            } else while (p < &value[list_len]) {
                  printf("%s\n", p);
                  p += strlen(p) + 1;
              }
          }
-    } else if (len < 0) {
+        zfree(value, val_len+1);
+    }
+    if (val_len < 0 || list_len < 0) {
          zwarnnam(nam, "%s: %e", metafy(*argv, slen, META_NOALLOC), errno);
-        ret = 1;
+        ret = 1 + (list_len > val_len);
      }
      return ret;
  }
-- 
1.6.5

[PATCH 4/4] attr: Use descriptive variables for argv and allow setting values with embedded nuls

[Mikael Magnusson]

---
  Src/Modules/attr.c |   63 ++++++++++++++++++++++++++-------------------------
  1 files changed, 32 insertions(+), 31 deletions(-)

diff --git a/Src/Modules/attr.c b/Src/Modules/attr.c
index bb30ebb..88ebb16 100644
--- a/Src/Modules/attr.c
+++ b/Src/Modules/attr.c
@@ -99,26 +99,26 @@ bin_getattr(char *nam, char **argv, Options ops, UNUSED(int func))
  {
      int ret = 0;
      int list_len, val_len, attr_len, slen;
-    char *value;
+    char *value, *file = argv[0], *attr = argv[1], *param = argv[2];
      int symlink = OPT_ISSET(ops, 'h');

-    unmetafy(*argv, &slen);
-    unmetafy(*(argv+1), NULL);
-    list_len = xlistxattr(*argv, NULL, 0, symlink);
+    unmetafy(file, &slen);
+    unmetafy(attr, NULL);
+    list_len = xlistxattr(file, NULL, 0, symlink);
      if (list_len > 0) {
-        val_len = xgetxattr(*argv, *(argv+1), NULL, 0, symlink);
+        val_len = xgetxattr(file, attr, NULL, 0, symlink);
          if (val_len == 0) {
-            if (*(argv+2))
-                unsetparam(*(argv+2));
+            if (param)
+                unsetparam(param);
              return 0;
          }
          if (val_len > 0) {
              value = (char *)zalloc(val_len+1);
-            attr_len = xgetxattr(*argv, *(argv+1), value, val_len, symlink);
+            attr_len = xgetxattr(file, attr, value, val_len, symlink);
              if (attr_len > 0 && attr_len <= val_len) {
                  value[attr_len] = '\0';
-                if (*(argv+2))
-                    setsparam(*(argv+2), metafy(value, attr_len, META_DUP));
+                if (param)
+                    setsparam(param, metafy(value, attr_len, META_DUP));
                  else
                      printf("%s\n", value);
              }
@@ -126,7 +126,7 @@ bin_getattr(char *nam, char **argv, Options ops, UNUSED(int func))
          }
      }
      if (list_len < 0 || val_len < 0 || attr_len < 0)  {
-        zwarnnam(nam, "%s: %e", metafy(*argv, slen, META_NOALLOC), errno);
+        zwarnnam(nam, "%s: %e", metafy(file, slen, META_NOALLOC), errno);
          ret = 1 + (attr_len > val_len);
      }
      return ret;
@@ -135,14 +135,15 @@ bin_getattr(char *nam, char **argv, Options ops, UNUSED(int func))
  static int
  bin_setattr(char *nam, char **argv, Options ops, UNUSED(int func))
  {
-    int ret = 0, slen;
+    int ret = 0, slen, vlen;
      int symlink = OPT_ISSET(ops, 'h');
+    char *file = argv[0], *attr = argv[1], *value = argv[2];

-    unmetafy(*argv, &slen);
-    unmetafy(*(argv+1), NULL);
-    unmetafy(*(argv+2), NULL);
-    if (xsetxattr(*argv, *(argv+1), *(argv+2), strlen(*(argv+2)), 0, symlink)) {
-        zwarnnam(nam, "%s: %e", metafy(*argv, slen, META_NOALLOC), errno);
+    unmetafy(file, &slen);
+    unmetafy(attr, NULL);
+    unmetafy(value, &vlen);
+    if (xsetxattr(file, attr, value, vlen, 0, symlink)) {
+        zwarnnam(nam, "%s: %e", metafy(file, slen, META_NOALLOC), errno);
          ret = 1;
      }
      return ret;
@@ -153,12 +154,12 @@ bin_delattr(char *nam, char **argv, Options ops, UNUSED(int func))
  {
      int ret = 0, slen;
      int symlink = OPT_ISSET(ops, 'h');
-    char *file = *argv;
+    char *file = argv[0], **attr = &argv[1];

      unmetafy(file, &slen);
-    while (*++argv) {
-        unmetafy(*argv, NULL);
-        if (xremovexattr(file, *argv, symlink)) {
+    while (*++attr) {
+        unmetafy(*attr, NULL);
+        if (xremovexattr(file, *attr, symlink)) {
              zwarnnam(nam, "%s: %e", metafy(file, slen, META_NOALLOC), errno);
              ret = 1;
              break;
@@ -172,24 +173,24 @@ bin_listattr(char *nam, char **argv, Options ops, UNUSED(int func))
  {
      int ret = 0;
      int val_len, list_len, slen;
-    char *value;
+    char *value, *file = argv[0], *param = argv[1];
      int symlink = OPT_ISSET(ops, 'h');

-    unmetafy(*argv, &slen);
-    val_len = xlistxattr(*argv, NULL, 0, symlink);
+    unmetafy(file, &slen);
+    val_len = xlistxattr(file, NULL, 0, symlink);
      if (val_len == 0) {
-        if (*(argv+1))
-            unsetparam(*(argv+1));
+        if (param)
+            unsetparam(param);
          return 0;
      }
      if (val_len > 0) {
          value = (char *)zalloc(val_len+1);
-        list_len = xlistxattr(*argv, value, val_len, symlink);
+        list_len = xlistxattr(file, value, val_len, symlink);
          if (list_len > 0 && list_len <= val_len) {
              char *p = value;
-            if (*(argv+1)) {
+            if (param) {
                  if (strlen(value) + 1 == list_len)
-                    setsparam(*(argv+1), metafy(value, list_len-1, META_DUP));
+                    setsparam(param, metafy(value, list_len-1, META_DUP));
                  else {
                      int arrlen = 0;
                      char **array = NULL, **arrptr = NULL;
@@ -204,7 +205,7 @@ bin_listattr(char *nam, char **argv, Options ops, UNUSED(int func))
                          *arrptr++ = metafy(p, -1, META_DUP);
                          p += strlen(p) + 1;
                      }
-                    setaparam(*(argv+1), array);
+                    setaparam(param, array);
                  }
              } else while (p < &value[list_len]) {
                  printf("%s\n", p);
@@ -214,7 +215,7 @@ bin_listattr(char *nam, char **argv, Options ops, UNUSED(int func))
          zfree(value, val_len+1);
      }
      if (val_len < 0 || list_len < 0) {
-        zwarnnam(nam, "%s: %e", metafy(*argv, slen, META_NOALLOC), errno);
+        zwarnnam(nam, "%s: %e", metafy(file, slen, META_NOALLOC), errno);
          ret = 1 + (list_len > val_len);
      }
      return ret;
-- 
1.6.5

Re: [PATCH 1/4] attr: add -h option to operate on symlinks without dereferencing

[Peter Stephenson]

On Tue, 3 Nov 2009 19:55:33 +0100 (CET)
Mikael Magnusson <mikachu@gmail.com> wrote:
> Since the OSX-style interface doesn't have the l* family, make wrapper
> functions that handle the boring details.

None of the hunks in any of the patches apply.  It's possible the white
space got scrwed up.  You can mail a complete diff as an attachment if you
like (I don't see any particular need to apply the four separately).

-- 
Peter Stephenson <pws@csr.com>            Software Engineer
Tel: +44 (0)1223 692070                   Cambridge Silicon Radio Limited
Churchill House, Cambridge Business Park, Cowley Road, Cambridge, CB4 0WZ, UK


Member of the CSR plc group of companies. CSR plc registered in England and Wales, registered number 4187346, registered office Churchill House, Cambridge Business Park, Cowley Road, Cambridge, CB4 0WZ, United Kingdom

Re: [PATCH 1/4] attr: add -h option to operate on symlinks without dereferencing

[Mikael Magnusson]

[-- Attachment #1: Type: text/plain, Size: 628 bytes --]

2009/11/4 Peter Stephenson <pws@csr.com>:
> On Tue, 3 Nov 2009 19:55:33 +0100 (CET)
> Mikael Magnusson <mikachu@gmail.com> wrote:
>> Since the OSX-style interface doesn't have the l* family, make wrapper
>> functions that handle the boring details.
>
> None of the hunks in any of the patches apply.  It's possible the white
> space got scrwed up.  You can mail a complete diff as an attachment if you
> like (I don't see any particular need to apply the four separately).

I only had a small hope it would work... oh well :). In the attached
patch I also added 2> /dev/null to zlistattr in the completer.

-- 
Mikael Magnusson

[-- Attachment #2: attr.diff --]
[-- Type: text/x-patch, Size: 9423 bytes --]

diff --git a/Completion/Zsh/Command/_zattr b/Completion/Zsh/Command/_zattr
index 56c754b..cdc5228 100644
--- a/Completion/Zsh/Command/_zattr
+++ b/Completion/Zsh/Command/_zattr
@@ -29,6 +29,6 @@ _arguments \
 esac
 
 if [[ $state = attrs ]]; then
-  zlistattr $~line[1] REPLY
-  _wanted attrs expl 'attribute' compadd ${(0)REPLY}
+  zlistattr $~line[1] REPLY 2> /dev/null
+  _wanted attrs expl 'attribute' compadd $REPLY
 fi
diff --git a/Doc/Zsh/mod_attr.yo b/Doc/Zsh/mod_attr.yo
index ed444d0..517bb63 100644
--- a/Doc/Zsh/mod_attr.yo
+++ b/Doc/Zsh/mod_attr.yo
@@ -32,3 +32,8 @@ var(filename). If the optional argument var(parameter) is given, the
 list of attributes is set on that parameter instead of being printed to stdout.
 )
 enditem()
+
+tt(zgetattr) and tt(zlistattr) allocate memory dynamically. If the attribute or
+list of attributes grows between the allocation and the call to get them, they
+return 2. On all other errors, 1 is returned. This way, the calling function can
+check for this case and try again for as long as they want.
diff --git a/Src/Modules/attr.c b/Src/Modules/attr.c
index ec3b1e4..88ebb16 100644
--- a/Src/Modules/attr.c
+++ b/Src/Modules/attr.c
@@ -33,102 +33,190 @@
 #include <sys/types.h>
 #include <sys/xattr.h>
 
-static int
-bin_getattr(char *nam, char **argv, UNUSED(Options ops), UNUSED(int func))
+static ssize_t
+xgetxattr(const char *path, const char *name, void *value, size_t size, int symlink)
 {
-    int ret = 0;
-    int len, slen;
-    char value[256];
+#ifdef XATTR_EXTRA_ARGS
+    return getxattr(path, name, value, size, 0, symlink ? XATTR_NOFOLLOW: 0);
+#else
+    switch (symlink) {
+    case 0:
+        return getxattr(path, name, value, size);
+    case 1:
+        return lgetxattr(path, name, value, size);
+    }
+#endif
+}
+
+static ssize_t
+xlistxattr(const char *path, char *list, size_t size, int symlink)
+{
+#ifdef XATTR_EXTRA_ARGS
+    return listxattr(path, list, size, symlink ? XATTR_NOFOLLOW : 0);
+#else
+    switch (symlink) {
+    case 0:
+        return listxattr(path, list, size);
+    case 1:
+        return llistxattr(path, list, size);
+    }
+#endif
+}
 
-    unmetafy(*argv, &slen);
-    unmetafy(*(argv+1), NULL);
-    if (listxattr(*argv, NULL, 0
+static int
+xsetxattr(const char *path, const char *name, const void *value,
+          size_t size, int flags, int symlink)
+{
 #ifdef XATTR_EXTRA_ARGS
-		  , 0
+    return setxattr(path, name, value, size, 0, flags | symlink ? XATTR_NOFOLLOW : 0);
+#else
+    switch (symlink) {
+    case 0:
+        return setxattr(path, name, value, size, flags);
+    case 1:
+        return lsetxattr(path, name, value, size, flags);
+    }
 #endif
-		  ) > 0) {
-        if (0 < (len = getxattr(*argv, *(argv+1), value, 255
+}
+
+static int
+xremovexattr(const char *path, const char *name, int symlink)
+{
 #ifdef XATTR_EXTRA_ARGS
-				, 0, 0
+    return removexattr(path, name, symlink ? XATTR_NOFOLLOW : 0);
+#else
+    switch (symlink) {
+    case 0:
+        return removexattr(path, name);
+    case 1:
+        return lremovexattr(path, name);
+    }
 #endif
-				))) {
-            if (len < 256) {
-                value[len] = '\0';
-                if (*(argv+2))
-                    setsparam(*(argv+2), metafy(value, len, META_DUP));
+}
+
+static int
+bin_getattr(char *nam, char **argv, Options ops, UNUSED(int func))
+{
+    int ret = 0;
+    int list_len, val_len, attr_len, slen;
+    char *value, *file = argv[0], *attr = argv[1], *param = argv[2];
+    int symlink = OPT_ISSET(ops, 'h');
+
+    unmetafy(file, &slen);
+    unmetafy(attr, NULL);
+    list_len = xlistxattr(file, NULL, 0, symlink);
+    if (list_len > 0) {
+        val_len = xgetxattr(file, attr, NULL, 0, symlink);
+        if (val_len == 0) {
+            if (param)
+                unsetparam(param);
+            return 0;
+        }
+        if (val_len > 0) {
+            value = (char *)zalloc(val_len+1);
+            attr_len = xgetxattr(file, attr, value, val_len, symlink);
+            if (attr_len > 0 && attr_len <= val_len) {
+                value[attr_len] = '\0';
+                if (param)
+                    setsparam(param, metafy(value, attr_len, META_DUP));
                 else
                     printf("%s\n", value);
             }
-        } else if (len < 0) {
-            zwarnnam(nam, "%s: %e", metafy(*argv, slen, META_NOALLOC), errno);
-            ret = 1;
+            zfree(value, val_len+1);
         }
     }
+    if (list_len < 0 || val_len < 0 || attr_len < 0)  {
+        zwarnnam(nam, "%s: %e", metafy(file, slen, META_NOALLOC), errno);
+        ret = 1 + (attr_len > val_len);
+    }
     return ret;
 }
 
 static int
-bin_setattr(char *nam, char **argv, UNUSED(Options ops), UNUSED(int func))
+bin_setattr(char *nam, char **argv, Options ops, UNUSED(int func))
 {
-    int ret = 0, slen;
+    int ret = 0, slen, vlen;
+    int symlink = OPT_ISSET(ops, 'h');
+    char *file = argv[0], *attr = argv[1], *value = argv[2];
 
-    unmetafy(*argv, &slen);
-    unmetafy(*(argv+1), NULL);
-    unmetafy(*(argv+2), NULL);
-    if (setxattr(*argv, *(argv+1), *(argv+2), strlen(*(argv+2)), 0
-#ifdef XATTR_EXTRA_ARGS
-						     , 0
-#endif
-		 )) {
-        zwarnnam(nam, "%s: %e", metafy(*argv, slen, META_NOALLOC), errno);
+    unmetafy(file, &slen);
+    unmetafy(attr, NULL);
+    unmetafy(value, &vlen);
+    if (xsetxattr(file, attr, value, vlen, 0, symlink)) {
+        zwarnnam(nam, "%s: %e", metafy(file, slen, META_NOALLOC), errno);
         ret = 1;
     }
     return ret;
 }
 
 static int
-bin_delattr(char *nam, char **argv, UNUSED(Options ops), UNUSED(int func))
+bin_delattr(char *nam, char **argv, Options ops, UNUSED(int func))
 {
     int ret = 0, slen;
+    int symlink = OPT_ISSET(ops, 'h');
+    char *file = argv[0], **attr = &argv[1];
 
-    unmetafy(*argv, &slen);
-    unmetafy(*(argv+1), NULL);
-    if (removexattr(*argv, *(argv+1)
-#ifdef XATTR_EXTRA_ARGS
-		    , 0
-#endif
-		    )) {
-        zwarnnam(nam, "%s: %e", metafy(*argv, slen, META_NOALLOC), errno);
-        ret = 1;
+    unmetafy(file, &slen);
+    while (*++attr) {
+        unmetafy(*attr, NULL);
+        if (xremovexattr(file, *attr, symlink)) {
+            zwarnnam(nam, "%s: %e", metafy(file, slen, META_NOALLOC), errno);
+            ret = 1;
+            break;
+        }
     }
     return ret;
 }
 
 static int
-bin_listattr(char *nam, char **argv, UNUSED(Options ops), UNUSED(int func))
+bin_listattr(char *nam, char **argv, Options ops, UNUSED(int func))
 {
     int ret = 0;
-    int len, slen;
-    char value[256];
+    int val_len, list_len, slen;
+    char *value, *file = argv[0], *param = argv[1];
+    int symlink = OPT_ISSET(ops, 'h');
 
-    unmetafy(*argv, &slen);
-    if (0 < (len = listxattr(*argv, value, 256
-#ifdef XATTR_EXTRA_ARGS
-		  , 0
-#endif
-			     ))) {
-        if (len < 256) {
+    unmetafy(file, &slen);
+    val_len = xlistxattr(file, NULL, 0, symlink);
+    if (val_len == 0) {
+        if (param)
+            unsetparam(param);
+        return 0;
+    }
+    if (val_len > 0) {
+        value = (char *)zalloc(val_len+1);
+        list_len = xlistxattr(file, value, val_len, symlink);
+        if (list_len > 0 && list_len <= val_len) {
             char *p = value;
-            if (*(argv+1))
-                setsparam(*(argv+1), metafy(value, len, META_DUP));
-            else while (p < &value[len]) {
+            if (param) {
+                if (strlen(value) + 1 == list_len)
+                    setsparam(param, metafy(value, list_len-1, META_DUP));
+                else {
+                    int arrlen = 0;
+                    char **array = NULL, **arrptr = NULL;
+
+                    while (p < &value[list_len]) {
+                        arrlen++;
+                        p += strlen(p) + 1;
+                    }
+                    arrptr = array = (char **)zshcalloc((arrlen+1) * sizeof(char *));
+                    p = value;
+                    while (p < &value[list_len]) {
+                        *arrptr++ = metafy(p, -1, META_DUP);
+                        p += strlen(p) + 1;
+                    }
+                    setaparam(param, array);
+                }
+            } else while (p < &value[list_len]) {
                 printf("%s\n", p);
                 p += strlen(p) + 1;
             }
         }
-    } else if (len < 0) {
-        zwarnnam(nam, "%s: %e", metafy(*argv, slen, META_NOALLOC), errno);
-        ret = 1;
+        zfree(value, val_len+1);
+    }
+    if (val_len < 0 || list_len < 0) {
+        zwarnnam(nam, "%s: %e", metafy(file, slen, META_NOALLOC), errno);
+        ret = 1 + (list_len > val_len);
     }
     return ret;
 }
@@ -136,10 +224,10 @@ bin_listattr(char *nam, char **argv, UNUSED(Options ops), UNUSED(int func))
 /* module paraphernalia */
 
 static struct builtin bintab[] = {
-    BUILTIN("zgetattr", 0, bin_getattr, 2, 3, 0, NULL, NULL),
-    BUILTIN("zsetattr", 0, bin_setattr, 3, 3, 0, NULL, NULL),
-    BUILTIN("zdelattr", 0, bin_delattr, 2, 2, 0, NULL, NULL),
-    BUILTIN("zlistattr", 0, bin_listattr, 1, 2, 0, NULL, NULL),
+    BUILTIN("zgetattr", 0, bin_getattr, 2, 3, 0, "h", NULL),
+    BUILTIN("zsetattr", 0, bin_setattr, 3, 3, 0, "h", NULL),
+    BUILTIN("zdelattr", 0, bin_delattr, 2, -1, 0, "h", NULL),
+    BUILTIN("zlistattr", 0, bin_listattr, 1, 2, 0, "h", NULL),
 };
 
 static struct features module_features = {

Re: [PATCH 1/4] attr: add -h option to operate on symlinks without dereferencing

[Peter Stephenson]

On Wed, 4 Nov 2009 12:01:08 +0100
Mikael Magnusson <mikachu@gmail.com> wrote:
> 2009/11/4 Peter Stephenson <pws@csr.com>:
> > On Tue, 3 Nov 2009 19:55:33 +0100 (CET)
> > Mikael Magnusson <mikachu@gmail.com> wrote:
> >> Since the OSX-style interface doesn't have the l* family, make wrapper
> >> functions that handle the boring details.
> >
> > None of the hunks in any of the patches apply.  It's possible the white
> > space got scrwed up.  You can mail a complete diff as an attachment if you
> > like (I don't see any particular need to apply the four separately).
> 
> I only had a small hope it would work... oh well :). In the attached
> patch I also added 2> /dev/null to zlistattr in the completer.

OK, I've committed this, but I haven't tried it out apart from compiling
it.

-- 
Peter Stephenson <pws@csr.com>            Software Engineer
Tel: +44 (0)1223 692070                   Cambridge Silicon Radio Limited
Churchill House, Cambridge Business Park, Cowley Road, Cambridge, CB4 0WZ, UK


Member of the CSR plc group of companies. CSR plc registered in England and Wales, registered number 4187346, registered office Churchill House, Cambridge Business Park, Cowley Road, Cambridge, CB4 0WZ, United Kingdom

Re: [PATCH 2/4] attr: Make zlistattr return an array, zdelattr takeseveral attrs

[Jun T.]

"zdelattr file attr1 attr2" deletes only attr2.

Index: Src/Modules/attr.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/Modules/attr.c,v
retrieving revision 1.4
diff -u -r1.4 attr.c
--- Src/Modules/attr.c	4 Nov 2009 11:34:03 -0000	1.4
+++ Src/Modules/attr.c	5 Nov 2009 05:17:33 -0000
@@ -154,7 +154,7 @@
 {
     int ret = 0, slen;
     int symlink = OPT_ISSET(ops, 'h');
-    char *file = argv[0], **attr = &argv[1];
+    char *file = argv[0], **attr = argv;
 
     unmetafy(file, &slen);
     while (*++attr) {

Re: [PATCH 2/4] attr: Make zlistattr return an array, zdelattr takeseveral attrs

[Mikael Magnusson]

2009/11/5 Jun T. <takimoto-j@kba.biglobe.ne.jp>:
> "zdelattr file attr1 attr2" deletes only attr2.
>
> Index: Src/Modules/attr.c
> ===================================================================
> RCS file: /cvsroot/zsh/zsh/Src/Modules/attr.c,v
> retrieving revision 1.4
> diff -u -r1.4 attr.c
> --- Src/Modules/attr.c  4 Nov 2009 11:34:03 -0000       1.4
> +++ Src/Modules/attr.c  5 Nov 2009 05:17:33 -0000
> @@ -154,7 +154,7 @@
>  {
>     int ret = 0, slen;
>     int symlink = OPT_ISSET(ops, 'h');
> -    char *file = argv[0], **attr = &argv[1];
> +    char *file = argv[0], **attr = argv;
>
>     unmetafy(file, &slen);
>     while (*++attr) {

Oops, I must have changed two things at once, I remember finding that
bug and fixing it. I must have broken it again after that. Thanks.

-- 
Mikael Magnusson

Re: [PATCH 1/4] attr: add -h option to operate on symlinks without dereferencing

[Mikael Magnusson]

On 3 November 2009 20:55, Mikael Magnusson <mikachu@gmail.com> wrote:
> Since the OSX-style interface doesn't have the l* family, make wrapper
> functions that handle the boring details.
> ---

It appears I forgot to document this. I'm not sure if the little blurb
about what -h does should be before or after the list of builtins or
if anyone cares, feel free to move it if you do :).

diff --git a/Doc/Zsh/mod_attr.yo b/Doc/Zsh/mod_attr.yo
index 96ac2e0..566e67f 100644
--- a/Doc/Zsh/mod_attr.yo
+++ b/Doc/Zsh/mod_attr.yo
@@ -2,31 +2,33 @@ COMMENT(!MOD!zsh/attr
 Builtins for manipulating extended attributes (xattr).
 !MOD!)
 The tt(zsh/attr) module is used for manipulating extended attributes.
+The tt(-h) option causes all commands to operate on symbolic links instead
+of their targets.
 The builtins in this module are:

 startitem()
 findex(zgetattr)
 cindex(extended attributes, xattr, getting from files)
-item(tt(zgetattr) var(filename) var(attribute) [ var(parameter) ])(
+item(tt(zgetattr) [ tt(-h) ] var(filename) var(attribute) [ var(parameter) ])(
 Get the extended attribute var(attribute) from the specified
 var(filename). If the optional argument var(parameter) is given, the
 attribute is set on that parameter instead of being printed to stdout.
 )
 findex(zsetattr)
 cindex(extended attributes, xattr, setting on files)
-item(tt(zsetattr) var(filename) var(attribute) var(value))(
+item(tt(zsetattr) [ tt(-h) ] var(filename) var(attribute) var(value))(
 Set the extended attribute var(attribute) on the specified
 var(filename) to var(value).
 )
 findex(zdelattr)
 cindex(extended attributes, xattr, removing, deleting)
-item(tt(zdelattr) var(filename) var(attribute))(
+item(tt(zdelattr) [ tt(-h) ] var(filename) var(attribute))(
 Remove the extended attribute var(attribute) from the specified
 var(filename).
 )
 findex(zlistattr)
 cindex(extended attributes, xattr, listing)
-item(tt(zlistattr) var(filename) [ var(parameter) ])(
+item(tt(zlistattr) [ tt(-h) ] var(filename) [ var(parameter) ])(
 List the extended attributes currently set on the specified
 var(filename). If the optional argument var(parameter) is given, the
 list of attributes is set on that parameter instead of being printed to stdout.



http://git.mika.l3ib.org/?p=zsh-cvs.git;a=patch;h=25f41c86ce79fdc1e07599fbd095f5fbf541ef7b

-- 
Mikael Magnusson