> On Jun 26, 2018, at 13:04, cgxu519 <cgxu519@xxxxxxx> wrote:
>
> Hi Ilya,
>
> Any objection for this? Maybe I should fix the overlapping return code as well.
> Should I split the patch to libceph and ceph part?
>
> On 06/24/2018 09:06 PM, Chengguang Xu wrote:
>> Using ceph_pagelist_encode_string() instead of combination of
>> ceph_pagelist_encode_32() and ceph_pagelist_append() when encoding
>> string. Meanwhile add return code check for ceph_pagelist_encode_string().
>>
>> Signed-off-by: Chengguang Xu <cgxu519@xxxxxxx>
>> ---
>> v2:
>> - Add return code check for ceph_pagelist_encode_string().
>>
>> fs/ceph/acl.c | 18 ++++++++++++------
>> net/ceph/osd_client.c | 16 +++++++++-------
>> 2 files changed, 21 insertions(+), 13 deletions(-)
>>
>> diff --git a/fs/ceph/acl.c b/fs/ceph/acl.c
>> index 5da752751a2b..6f0210984456 100644
>> --- a/fs/ceph/acl.c
>> +++ b/fs/ceph/acl.c
>> @@ -222,14 +222,17 @@ int ceph_pre_init_acls(struct inode *dir, umode_t *mode,
>> err = ceph_pagelist_reserve(pagelist, len + val_size1 + 8);
>> if (err)
>> goto out_err;
>> - ceph_pagelist_encode_string(pagelist, XATTR_NAME_POSIX_ACL_ACCESS,
>> - len);
>> + err = ceph_pagelist_encode_string(pagelist,
>> + XATTR_NAME_POSIX_ACL_ACCESS, len);
>> + if (err)
>> + goto out_err;
>> err = posix_acl_to_xattr(&init_user_ns, acl,
>> tmp_buf, val_size1);
>> if (err < 0)
>> goto out_err;
>> - ceph_pagelist_encode_32(pagelist, val_size1);
>> - ceph_pagelist_append(pagelist, tmp_buf, val_size1);
>> + err = ceph_pagelist_encode_string(pagelist, tmp_buf, val_size1);
>> + if (err)
>> + goto out_err;
>> }
the code first reserve space, then add data to page list. no need to check return
>> if (default_acl) {
>> size_t len = strlen(XATTR_NAME_POSIX_ACL_DEFAULT);
>> @@ -238,12 +241,15 @@ int ceph_pre_init_acls(struct inode *dir, umode_t *mode,
>> goto out_err;
>> err = ceph_pagelist_encode_string(pagelist,
>> XATTR_NAME_POSIX_ACL_DEFAULT, len);
>> + if (err)
>> + goto out_err;
>> err = posix_acl_to_xattr(&init_user_ns, default_acl,
>> tmp_buf, val_size2);
>> if (err < 0)
>> goto out_err;
>> - ceph_pagelist_encode_32(pagelist, val_size2);
>> - ceph_pagelist_append(pagelist, tmp_buf, val_size2);
>> + err = ceph_pagelist_encode_string(pagelist, tmp_buf, val_size2);
>> + if (err)
>> + goto out_err;
>> }
>> kfree(tmp_buf);
>> diff --git a/net/ceph/osd_client.c b/net/ceph/osd_client.c
>> index f2584fe1246f..720ac564d392 100644
>> --- a/net/ceph/osd_client.c
>> +++ b/net/ceph/osd_client.c
>> @@ -4607,14 +4607,15 @@ static int osd_req_op_notify_ack_init(struct ceph_osd_request *req, int which,
>> ceph_pagelist_init(pl);
>> ret = ceph_pagelist_encode_64(pl, notify_id);
>> ret |= ceph_pagelist_encode_64(pl, cookie);
>> - if (payload) {
>> - ret |= ceph_pagelist_encode_32(pl, payload_len);
>> - ret |= ceph_pagelist_append(pl, payload, payload_len);
>> - } else {
>> + if (payload)
>> + ret |= ceph_pagelist_encode_string(pl, payload, payload_len);
>> + else
>> ret |= ceph_pagelist_encode_32(pl, 0);
>> - }
>> +
>> if (ret) {
>> ceph_pagelist_release(pl);
>> + if (ret & -ERANGE)
>> + return -ERANGE;
>> return -ENOMEM;
>> }
>> @@ -4678,10 +4679,11 @@ static int osd_req_op_notify_init(struct ceph_osd_request *req, int which,
>> ceph_pagelist_init(pl);
>> ret = ceph_pagelist_encode_32(pl, 1); /* prot_ver */
>> ret |= ceph_pagelist_encode_32(pl, timeout);
>> - ret |= ceph_pagelist_encode_32(pl, payload_len);
>> - ret |= ceph_pagelist_append(pl, payload, payload_len);
>> + ret |= ceph_pagelist_encode_string(pl, payload, payload_len);
>> if (ret) {
>> ceph_pagelist_release(pl);
>> + if (ret & -ERANGE)
>> + return -ERANGE;
>> return -ENOMEM;
>> }
>>
>
--
To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
