[PATCH] getfattr: new applet
David Leonard
d+busybox at adaptive-enterprises.com
Thu Jul 6 03:15:24 UTC 2023
On Mon, 3 Jul 2023, 余生与君 wrote:
> Gentle ping.
>
> On Sun, Jun 18, 2023 at 4:53 PM LoveSy <lovesykun at gmail.com> wrote:
>>
>> function old new delta
>> getfattr_main - 380 +380
>> print_attr - 204 +204
>> list_attr - 152 +152
>> .rodata 95358 95401 +43
>> applet_names 2766 2775 +9
>> e843419 at 0048_000003ed_14 - 8 +8
>> e843419 at 0047_000003d1_550 - 8 +8
>> applet_main 3216 3224 +8
>> packed_usage 34560 34567 +7
>> applet_install_loc 201 202 +1
>> ------------------------------------------------------------------------------
>> (add/remove: 6/0 grow/shrink: 5/0 up/down: 820/0) Total: 820 bytes
>> text data bss dec hex filename
>> 1127717 16889 1736 1146342 117de6 busybox_old
>> 1141124 16937 1736 1159797 11b275 busybox_unstripped
>>
>> Signed-off-by: LoveSy <lovesykun at gmail.com>
>> ---
>> miscutils/getfattr.c | 122 +++++++++++++++++++++++++++++++++++++++++++
>> 1 file changed, 122 insertions(+)
>> create mode 100644 miscutils/getfattr.c
>>
>> diff --git a/miscutils/getfattr.c b/miscutils/getfattr.c
>> new file mode 100644
>> index 000000000..4d4eacb5f
>> --- /dev/null
>> +++ b/miscutils/getfattr.c
>> @@ -0,0 +1,122 @@
>> +/*
>> + * getfattr - get extended attributes of filesystem objects.
>> + *
>> + * Copyright (C) 2023 by LoveSy <lovesykun at gmail.com>
>> + *
>> + * Licensed under GPLv2, see file LICENSE in this source tree.
>> + */
>> +//config:config GETFATTR
>> +//config: bool "getfattr (12.3 kb)"
>> +//config: default y
>> +//config: help
>> +//config: Get extended attributes on files
>> +
>> +//applet:IF_GETFATTR(APPLET_NOEXEC(getfattr, getfattr, BB_DIR_USR_BIN, BB_SUID_DROP, getfattr))
>> +
>> +//kbuild:lib-$(CONFIG_GETFATTR) += getfattr.o
>> +
>> +#include <stdio.h>
>> +#include <sys/xattr.h>
>> +#include "libbb.h"
>> +
>> +//usage:#define getfattr_trivial_usage
>> +//usage: "[-h] -n|-d ATTR FILE..."
>> +//usage:#define getfattr_full_usage "\n\n"
>> +//usage: "Get extended attributes"
>> +//usage: "\n"
>> +//usage: "\n -h Do not follow symlinks"
>> +//usage: "\n -d Dump all attributes"
>> +//usage: "\n -n ATTR Get attribute ATTR"
Hi,
The usage string is a bit confusing. The libattr tools' usages are online as:
getfattr [-hRLP] -n name [-e en] pathname...
getfattr [-hRLP] -d [-e en] [-m pattern] pathname...
setfattr [-h] -n name [-v value] pathname...
setfattr [-h] -x name pathname...
setfattr [-h] --restore=file
And the existing busybox setfattr is
setfattr [-h] -n|-x ATTR [-v VALUE] FILE...
Which I suppose makes more sense if you read it as
setfattr [-h] {-n|-x} ATTR [-v VALUE] FILE...
Here, in getfattr, perhaps you could steal the trivial usage pattern from
mv.c and sed.c and write it like this?
//usage: #define getfattr_trivial_usage
//usage: "[-h] -n name FILE...\n"
//usage: "or: getfattr [-h] -d FILE..."
>> +int getfattr_main(int argc, char **argv) MAIN_EXTERNALLY_VISIBLE;
>> +
>> +int print_attr(const char *file, const char *name, int follow, char **buf, size_t *buflen)
>> +{
>> + ssize_t vallen, gotlen;
>> + vallen = (follow ? getxattr: lgetxattr)(file, name, NULL, 0);
>> + if (vallen == -1) {
>> + return -1;
>> + }
>> + vallen += 1;
>> + if (*buflen < vallen) {
>> + *buf = (char *)xrealloc(*buf, vallen);
The cast should be unnecessary. Also for another xrealloc later.
>> + *buflen = vallen;
>> + }
>> + vallen = (follow ? getxattr : lgetxattr)(file, name, *buf, vallen);
Probably should pass arg 4 as *buflen. Also, similar for [l]listxattr later.
>> + if (vallen == -1) {
>> + return -1;
>> + }
>> + (*buf)[vallen] = '\0';
>> + printf("%s=\"%s\"\n", name, *buf);
Could avoid +1 and '\0' termination with:
printf("%s=\"%.*s\"\n", name, vallen, *buf);
>> + return 0;
>> +}
>> +
>> +ssize_t list_attr(const char *file, int follow, char **list, size_t *listlen)
>> +{
>> + ssize_t len;
>> + len = (follow ? listxattr : llistxattr)(file, NULL, 0);
>> + if (len == -1) {
>> + return -1;
>> + }
>> + if (*listlen < len) {
>> + *list = (char *)xrealloc(*list, len);
>> + *listlen = len;
>> + }
>> + len = (follow ? listxattr : llistxattr)(file, *list, len);
>> + if (len == -1) {
>> + return -1;
>> + }
>> + return len;
>> +}
>> +
>> +int getfattr_main(int argc UNUSED_PARAM, char **argv)
>> +{
>> + const char *name = NULL;
>> + int status;
>> + int opt;
>> + char *buf = NULL;
>> + size_t buflen = 0;
>> + char *list = NULL;
>> + size_t listlen = 0;
Small nitpick: buflen and listlen (and keylen) are sizes, not lengths.
It would align with the syscall parameter names ("size") in the manual pages.
>> + enum {
>> + OPT_h = (1 << 0),
>> + OPT_d = (1 << 1),
>> + };
>> +
>> + opt = getopt32(argv, "^"
>> + "hdn:"
>> + /* Min one arg, either -x or -n is a must, -d does not allow -n */
>> + "\0" "-1:dn:n--d:d--n"
The -x in the comment should be -d?
Also, I suspect the ':dn:' component should be :d:n:, i.e.:
/* Min one arg; exactly one of -n or -d is required. */
"\0" "-1:d:n:n--d:d--n"
I'm unsure but I based this on the example in getopt32.c which says:
// Don't allow -KS -SK, but -S or -K is required
flags = getopt32(argv, "^KS...""\0""K:S:K--S:S--K");
>> + , &name
>> + );
>> + argv += optind;
>> + status = EXIT_SUCCESS;
>> +
>> + do {
>> + int r = 0;
>> + printf("# file: %s\n", *argv);
>> + if (opt & OPT_d) {
>> + ssize_t len = list_attr(*argv, !(opt & OPT_h), &list, &listlen);
>> + ssize_t keylen;
>> + char *key = list;
>> + while (len > 0 && !r) {
>> + r = print_attr(*argv, key, !(opt & OPT_h), &buf, &buflen);
>> + keylen = strlen(key) + 1;
>> + key += keylen;
>> + len -= keylen;
>> + }
>> + }
>> + else {
>> + r = print_attr(*argv, name, !(opt & OPT_h), &buf, &buflen);
>> + }
>> + printf("\n");
>> +
>> + if (r) {
>> + bb_simple_perror_msg(*argv);
>> + status = EXIT_FAILURE;
>> + }
>> + } while (*++argv);
>> +
>> + free(buf);
>> +
>> + return status;
>> +}
>> --
>> 2.34.1
>>
> _______________________________________________
> busybox mailing list
> busybox at busybox.net
> http://lists.busybox.net/mailman/listinfo/busybox
More information about the busybox
mailing list