Re: [RFC PATCH 17/35] libceph, rbd: Use ceph_databuf encoding start/stop

[David Howells]

Viacheslav Dubeyko <Slava.Dubeyko@xxxxxxx> wrote:

> > -		ceph_encode_string(&p, end, buf, len);
> > +		BUG_ON(p + sizeof(__le32) + len > end);
> 
> Frankly speaking, it's hard to follow why sizeof(__le32) should be in the
> equation. Maybe, it make sense to introduce some constant? The name of
> constant makes understanding of this calculation more clear.

Look through the patch.  It's done all over the place, even on parts I haven't
touched.  However, it's probably because of the way the string is encoded
(4-byte LE length followed by the characters).

It probably would make sense to use a calculation wrapper for this.  I have
this in fs/afs/yfsclient.c for example:

	static size_t xdr_strlen(unsigned int len)
	{
		return sizeof(__be32) + round_up(len, sizeof(__be32));
	}

> > +	BUG_ON(sizeof(__le64) + sizeof(__le32) + wsize > req->request->front_alloc_len);
> 
> The same problem is here. It's hard to follow to this check by involving
> sizeof(__le64) and sizeof(__le32) in calculation. What these numbers mean here?

Presumably the sizes of the protocol elements in the marshalled data.  If you
want to clean all those up in some way, I can add your patch into my
series;-).

David