ocfs2: using the OCFS2_XATTR_ROOT_SIZE macro in ocfs2_reflink_xattr_header()

PATCH Applied alex chen 2 participants 3 messages Dec 11, 2017 – Dec 13, 2017 OCFS2

Share & tools

Andrew Morton

2017-12-12 22:47 UTC #1 lore raw

PATCH v2 1/1 ocfs2: using the OCFS2_XATTR_ROOT_SIZE macro in ocfs2_reflink_xattr_header()

Quoted 13 lines · On Mon, 11 Dec 2017 14:24:08 +0800 alex chen <[email protected]> wrote:

Using the OCFS2_XATTR_ROOT_SIZE macro improves the readability of the code.
Signed-off-by Alex Chen <[email protected]>
Reviewed-by Jun Piao <[email protected]>
---
 fs/ocfs2/xattr.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
fs/ocfs2/xattr.c +0 -0
index 5fdf269..ca3b61a 100644

fs/ocfs2/xattr.c static int ocfs2_reflink_xattr_header(handle_t *handle, · line 6421

Quoted 8 lines

fs/ocfs2/xattr.c +1 -1

@@ -6415,7 +6415,7 @@ static int ocfs2_reflink_xattr_header(handle_t *handle,
 		 * and then insert the extents one by one.
 		 */
 		if (xv->xr_list.l_tree_depth) {
-			memcpy(new_xv, &def_xv, sizeof(def_xv));
+			memcpy(new_xv, &def_xv, OCFS2_XATTR_ROOT_SIZE);
 			vb->vb_xv = new_xv;
 			vb->vb_bh = value_bh;
 			ocfs2_init_xattr_value_extent_tree(&data_et,

OK.

But what's wrong with

	*new_xv = def_xv;

That gets typechecked and the compiler may be able to perform some optimizations...

alex chen

author

2017-12-13 07:49 UTC #2 lore raw

PATCH v2 1/1 ocfs2: using the OCFS2_XATTR_ROOT_SIZE macro in ocfs2_reflink_xattr_header()

Hi Andrew,

Thanks for your suggestion.

Quoted 26 lines · Andrew Morton

Quoted 13 lines · On Mon, 11 Dec 2017 14:24:08 +0800 alex chen <[email protected]> wrote:
Using the OCFS2_XATTR_ROOT_SIZE macro improves the readability of the code.
Signed-off-by Alex Chen <[email protected]>
Reviewed-by Jun Piao <[email protected]>
---
 fs/ocfs2/xattr.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
fs/ocfs2/xattr.c +0 -0
index 5fdf269..ca3b61a 100644
fs/ocfs2/xattr.c static int ocfs2_reflink_xattr_header(handle_t *handle, · line 6421
Quoted 8 lines
fs/ocfs2/xattr.c +1 -1
@@ -6415,7 +6415,7 @@ static int ocfs2_reflink_xattr_header(handle_t *handle,
 		 * and then insert the extents one by one.
 		 */
 		if (xv->xr_list.l_tree_depth) {
-			memcpy(new_xv, &def_xv, sizeof(def_xv));
+			memcpy(new_xv, &def_xv, OCFS2_XATTR_ROOT_SIZE);
 			vb->vb_xv = new_xv;
 			vb->vb_bh = value_bh;
 			ocfs2_init_xattr_value_extent_tree(&data_et,
OK.

Quoted 5 lines

But what's wrong with
	*new_xv = def_xv;
?

The type of new_xv is 'ocfs2_xattr_value_root' and the type of def_xv is 'ocfs2_xattr_def_value_root'. The length of def_xv is larger than that of new_xv.
We initialize the new_xv to the empty default value root which have one extent record. If we use method you describe above to copy, we may missed a copy of one extent record.

Thanks,
Alex

Quoted 7 lines

That gets typechecked and the compiler may be able to perform some optimizations...
.