On Wed, Jul 11, 2012 at 12:14 PM, Alex Elder <elder@xxxxxxxxxxx> wrote:
> 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?
Yeah. There are several examples of us doing it (look at ceph_decode_need).
>
> Should I assume that applies to the previous patch also?
Right. Good call.
Yehuda
--
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
