On 07/11/2012 12:20 PM, Yehuda Sadeh wrote:
> On Wed, Jul 11, 2012 at 7:01 AM, Alex Elder <elder@xxxxxxxxxxx> wrote:
>> This adds a new utility routine which will return a dynamically-
>> allocated buffer containing a string that has been decoded from ceph
>> over-the-wire format. It also returns the length of the string
>> if the address of a size variable is supplied to receive it.
>>
>> For now, no gfp_flags parameter is defined (GFP_KERNEL is used) but
>> it could be easily be added if needed.
>>
>
> I'd rather have it upfront, will help avoiding future errors.
I actually wanted to do exactly that before I sent it but I
forgot. I guess I got all caught up in the excitement.
>> Signed-off-by: Alex Elder <elder@xxxxxxxxxxx>
>> ---
>> include/linux/ceph/decode.h | 29 +++++++++++++++++++++++++++++
>> 1 files changed, 29 insertions(+), 0 deletions(-)
>>
>> diff --git a/include/linux/ceph/decode.h b/include/linux/ceph/decode.h
>> index 7ead11fc..7759164 100644
>> --- a/include/linux/ceph/decode.h
>> +++ b/include/linux/ceph/decode.h
>> @@ -80,6 +80,35 @@ static inline size_t ceph_decode_string(void **p,
>> char *s, size_t size)
>> }
>>
>> /*
>> + * Allocate a buffer big enough to hold the wire-encoded string, and
>> + * decode the string into it. The resulting string will always be
>> + * terminated with '\0'. If successful, *p will be advanced
>> + * past the decoded data. Also, if lenp is not a null pointer, the
>> + * length (not including the terminating '\0') will be recorded in
>> + * it. Note that a zero-length string is a valid return value.
>> + *
>> + * Returns a pointer to the newly-allocated string buffer, or a
>> + * null pointer if memory could not be allocated for the result.
>> + * Neither of the arguments is updated if NULL is returned.
>> + */
>> +static inline char *ceph_extract_encoded_string(void **p, size_t *lenp)
>> +{
>> + size_t len;
>> + char *buf;
>> +
>> + len = ceph_decode_string(p, NULL, 0);
>> + buf = kmalloc(len + 1, GFP_KERNEL);
>> + if (!buf)
>> + return NULL;
>> +
>> + (void) ceph_decode_string(p, buf, len + 1);
>> + if (lenp)
>> + *lenp = len;
>> +
>> + return buf;
>> +}
>> +
>> +/*
>
> We don't make an effort here to check whether encoded string buffer is
> valid. While we may be checking it somewhere up the stack, this seem
> like a generic enough function that could be naively used. Either make
> it clear that it's an internal function, or make it check p bounds.
Are you saying I should have the caller provide the length of the
buffer, and ensure we don't exceed it?
Should I assume that applies to the previous patch also?
In any case, I will rework it, but I want to make sure I understand
what you're saying.
-Alex
>> * bounds check input.
>> */
>> static inline int ceph_has_room(void **p, void *end, size_t n)
>> --
>> 1.7.5.4
>>
>> --
>> 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
>
>
--
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
