Re: [wip patch] new zsh/attr module

[Mikael Magnusson <mikachu@gmail.com>]

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