diff mbox

[v3,04/16] libceph: define ceph_extract_encoded_string()

Message ID 4FFF05D4.9080006@inktank.com (mailing list archive)
State New, archived
Headers show

Commit Message

Alex Elder July 12, 2012, 5:13 p.m. UTC
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.

Signed-off-by: Alex Elder <elder@inktank.com>
---
v3: Final version.  Sage convinced me that there was no need for
    ceph_decode_string() other than its use in this function, so
    now this implements what that function had been doing directly.

 include/linux/ceph/decode.h |   44
++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 44 insertions(+)

--
To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Sage Weil July 12, 2012, 6:20 p.m. UTC | #1
On Thu, 12 Jul 2012, Alex Elder 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.
> 
> Signed-off-by: Alex Elder <elder@inktank.com>
> ---
> v3: Final version.  Sage convinced me that there was no need for
>     ceph_decode_string() other than its use in this function, so
>     now this implements what that function had been doing directly.
> 
>  include/linux/ceph/decode.h |   44
> ++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 44 insertions(+)
> 
> Index: b/include/linux/ceph/decode.h
> ===================================================================
> --- a/include/linux/ceph/decode.h
> +++ b/include/linux/ceph/decode.h
> @@ -85,6 +85,50 @@ static inline int ceph_has_room(void **p
>  	} while (0)
> 
>  /*
> + * 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
> + * *lenp.  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 an error occurs.  Neither *p nor *lenp will have been
> + * updated if NULL is returned.
> + *
> + * There are two possible failures:
> + *   - converting the string would require accessing memory at or
> + *     beyond the "end" pointer provided
> + *   - memory could not be allocated for the result

I think the caller should be able to distinguish between these two cases 
in the return value.  What about:

> + */
> +static inline char *ceph_extract_encoded_string(void **p, void *end,
> +						size_t *lenp, gfp_t gfp)


int ceph_extract_encoded_string(void **p, void *end, char **str, gfp_t gfp)

and return the length, or an error code?  That avoids futzing with ERR_PTR 
and gives you the len a bit less awkwardly...

sage


> +{
> +	u32 len;
> +	void *sp = *p;
> +	char *buf = NULL;
> +
> +	ceph_decode_32_safe(&sp, end, len, out);
> +	if (!ceph_has_room(&sp, end, len))
> +		return NULL;
> +
> +	buf = kmalloc(len + 1, gfp);
> +	if (!buf)
> +		return NULL;
> +
> +	if (len)
> +		memcpy(buf, sp, len);
> +	buf[len] = '\0';
> +
> +	*p = (char *) *p + sizeof (u32) + len;
> +
> +	if (lenp)
> +		*lenp = (size_t) len;
> +out:
> +	return buf;
> +}
> +
> +/*
>   * struct ceph_timespec <-> struct timespec
>   */
>  static inline void ceph_decode_timespec(struct timespec *ts,
> --
> To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
> the body of a message to majordomo@vger.kernel.org
> 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@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Alex Elder July 12, 2012, 7:48 p.m. UTC | #2
On 07/12/2012 01:20 PM, Sage Weil wrote:
> I think the caller should be able to distinguish between these two cases 
> in the return value. 

OK.  I'll adjust and repost.	-Alex
--
To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Alex Elder July 12, 2012, 10:47 p.m. UTC | #3
On 07/12/2012 01:20 PM, Sage Weil wrote:
> I think the caller should be able to distinguish between these two cases 
> in the return value.  What about:
> 
>> > + */
>> > +static inline char *ceph_extract_encoded_string(void **p, void *end,
>> > +						size_t *lenp, gfp_t gfp)
> 
> int ceph_extract_encoded_string(void **p, void *end, char **str, gfp_t gfp)
> 
> and return the length, or an error code?  That avoids futzing with ERR_PTR 
> and gives you the len a bit less awkwardly...
> 

I opted to stick with my original prototype--returning the string--and
using the ERR_PTR() etc.  I return the string because I *always* want
that.  Only sometimes is the length needed.

					-Alex

--
To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

Index: b/include/linux/ceph/decode.h
===================================================================
--- a/include/linux/ceph/decode.h
+++ b/include/linux/ceph/decode.h
@@ -85,6 +85,50 @@  static inline int ceph_has_room(void **p
 	} while (0)

 /*
+ * 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
+ * *lenp.  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 an error occurs.  Neither *p nor *lenp will have been
+ * updated if NULL is returned.
+ *
+ * There are two possible failures:
+ *   - converting the string would require accessing memory at or
+ *     beyond the "end" pointer provided
+ *   - memory could not be allocated for the result
+ */
+static inline char *ceph_extract_encoded_string(void **p, void *end,
+						size_t *lenp, gfp_t gfp)
+{
+	u32 len;
+	void *sp = *p;
+	char *buf = NULL;
+
+	ceph_decode_32_safe(&sp, end, len, out);
+	if (!ceph_has_room(&sp, end, len))
+		return NULL;
+
+	buf = kmalloc(len + 1, gfp);
+	if (!buf)
+		return NULL;
+
+	if (len)
+		memcpy(buf, sp, len);
+	buf[len] = '\0';
+
+	*p = (char *) *p + sizeof (u32) + len;
+
+	if (lenp)
+		*lenp = (size_t) len;
+out:
+	return buf;
+}
+
+/*
  * struct ceph_timespec <-> struct timespec
  */
 static inline void ceph_decode_timespec(struct timespec *ts,