| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20170724033050.fpkwpyywelldzshi@thunk.org>
Date: Sun, 23 Jul 2017 23:30:50 -0400
From: Theodore Ts'o <tytso@....edu>
To: Tahsin Erdogan <tahsin@...gle.com>
Cc: Andreas Dilger <adilger@...ger.ca>,
"Darrick J . Wong" <darrick.wong@...cle.com>,
linux-ext4@...r.kernel.org
Subject: Re: [PATCH 4/4] libext2fs: add ea_inode support to set xattr
Applied, with some style cleanups. See below for the interdiff.
The cleanups are mostly checkpatch style warnings, plus simplifying
complexity by reducing indentation levels. Yes, it makes the code
somewhat less parallel, but on the whole I think it makes the code
more readable.
- Ted
lib/ext2fs/ext_attr.c | 112 +++++++++++++++++++++------------------------
1 file changed, 52 insertions(+), 60 deletions(-)
diff --git a/lib/ext2fs/ext_attr.c b/lib/ext2fs/ext_attr.c
index 7cecb1688..f4cc2d056 100644
--- a/lib/ext2fs/ext_attr.c
+++ b/lib/ext2fs/ext_attr.c
@@ -1334,7 +1334,7 @@ static errcode_t xattr_update_entry(ext2_filsys fs, struct ext2_xattr *x,
if (!x->name)
x->name = new_name;
-
+
if (x->value)
ext2fs_free_mem(&x->value);
x->value = new_value;
@@ -1351,7 +1351,8 @@ fail:
return ret;
}
-static int xattr_find_position(struct ext2_xattr *attrs, int count, const char *name)
+static int xattr_find_position(struct ext2_xattr *attrs, int count,
+ const char *name)
{
struct ext2_xattr *x;
int i;
@@ -1400,7 +1401,7 @@ errcode_t xattr_array_update(struct ext2_xattr_handle *h, const char *name,
if (!in_inode)
needed += EXT2_EXT_ATTR_SIZE(value_len);
- if (0 <= old_idx && old_idx < h->ibody_count) {
+ if (old_idx >= 0 && old_idx < h->ibody_count) {
ibody_free += EXT2_EXT_ATTR_LEN(name_len);
if (!h->attrs[old_idx].ea_ino)
ibody_free += EXT2_EXT_ATTR_SIZE(
@@ -1408,75 +1409,66 @@ errcode_t xattr_array_update(struct ext2_xattr_handle *h, const char *name,
}
if (needed <= ibody_free) {
- if (0 <= old_idx) {
- /* Update the existing entry. */
- ret = xattr_update_entry(h->fs, &h->attrs[old_idx],
- name, value, value_len,
- in_inode);
- if (ret)
- return ret;
- if (h->ibody_count <= old_idx) {
- /* Move entry from block to the end of ibody. */
- tmp = h->attrs[old_idx];
- memmove(h->attrs + h->ibody_count + 1,
- h->attrs + h->ibody_count,
- (old_idx - h->ibody_count)
- * sizeof(*h->attrs));
- h->attrs[h->ibody_count] = tmp;
- h->ibody_count++;
- }
- return 0;
- } else {
+ if (old_idx < 0) {
new_idx = h->ibody_count;
add_to_ibody = 1;
goto add_new;
}
+
+ /* Update the existing entry. */
+ ret = xattr_update_entry(h->fs, &h->attrs[old_idx], name,
+ value, value_len, in_inode);
+ if (ret)
+ return ret;
+ if (h->ibody_count <= old_idx) {
+ /* Move entry from block to the end of ibody. */
+ tmp = h->attrs[old_idx];
+ memmove(h->attrs + h->ibody_count + 1,
+ h->attrs + h->ibody_count,
+ (old_idx - h->ibody_count) * sizeof(*h->attrs));
+ h->attrs[h->ibody_count] = tmp;
+ h->ibody_count++;
+ }
+ return 0;
}
if (h->ibody_count <= old_idx) {
block_free += EXT2_EXT_ATTR_LEN(name_len);
if (!h->attrs[old_idx].ea_ino)
- block_free += EXT2_EXT_ATTR_SIZE(
- h->attrs[old_idx].value_len);
+ block_free +=
+ EXT2_EXT_ATTR_SIZE(h->attrs[old_idx].value_len);
}
- if (needed <= block_free) {
- if (0 <= old_idx) {
- /* Update the existing entry. */
- ret = xattr_update_entry(h->fs, &h->attrs[old_idx],
- name, value, value_len,
- in_inode);
- if (ret)
- return ret;
- if (old_idx < h->ibody_count) {
- /*
- * Move entry from ibody to the block. Note that
- * entries in the block are sorted.
- */
- new_idx = xattr_find_position(
- h->attrs + h->ibody_count,
- h->count - h->ibody_count,
- name);
- new_idx += h->ibody_count - 1;
- tmp = h->attrs[old_idx];
- memmove(h->attrs + old_idx,
- h->attrs + old_idx + 1,
- (new_idx - old_idx)*sizeof(*h->attrs));
- h->attrs[new_idx] = tmp;
- h->ibody_count--;
- }
- return 0;
- } else {
+ if (needed > block_free)
+ return EXT2_ET_EA_NO_SPACE;
+
+ if (old_idx >= 0) {
+ /* Update the existing entry. */
+ ret = xattr_update_entry(h->fs, &h->attrs[old_idx], name,
+ value, value_len, in_inode);
+ if (ret)
+ return ret;
+ if (old_idx < h->ibody_count) {
+ /*
+ * Move entry from ibody to the block. Note that
+ * entries in the block are sorted.
+ */
new_idx = xattr_find_position(h->attrs + h->ibody_count,
- h->count - h->ibody_count,
- name);
- new_idx += h->ibody_count;
- add_to_ibody = 0;
- goto add_new;
+ h->count - h->ibody_count, name);
+ new_idx += h->ibody_count - 1;
+ tmp = h->attrs[old_idx];
+ memmove(h->attrs + old_idx, h->attrs + old_idx + 1,
+ (new_idx - old_idx) * sizeof(*h->attrs));
+ h->attrs[new_idx] = tmp;
+ h->ibody_count--;
}
+ return 0;
}
- return EXT2_ET_EA_NO_SPACE;
+ new_idx = xattr_find_position(h->attrs + h->ibody_count,
+ h->count - h->ibody_count, name);
+ new_idx += h->ibody_count;
+ add_to_ibody = 0;
add_new:
if (h->count == h->capacity) {
@@ -1520,7 +1512,7 @@ int space_used(struct ext2_xattr *attrs, int count)
/*
* The minimum size of EA value when you start storing it in an external inode
* size of block - size of header - size of 1 entry - 4 null bytes
-*/
+ */
#define EXT4_XATTR_MIN_LARGE_EA_SIZE(b) \
((b) - EXT2_EXT_ATTR_LEN(3) - sizeof(struct ext2_ext_attr_header) - 4)
@@ -1579,8 +1571,8 @@ errcode_t ext2fs_xattr_set(struct ext2_xattr_handle *h,
if (inode_size > EXT2_GOOD_OLD_INODE_SIZE) {
extra_isize = inode->i_extra_isize;
if (extra_isize == 0) {
- extra_isize = fs->super->s_want_extra_isize;
- if (extra_isize == 0)
+ extra_isize = fs->super->s_want_extra_isize;
+ if (extra_isize == 0)
extra_isize = sizeof(__u32);
}
ibody_free = inode_size - EXT2_GOOD_OLD_INODE_SIZE;
Powered by blists - more mailing lists