On Tue, 2025-03-18 at 22:19 +0000, David Howells wrote:
Viacheslav Dubeyko [off-list ref] wrote:
quoted
quoted
- 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));
}
quoted
quoted
+ 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;-).
Yeah, I am considering to make the likewise cleanup. :)
Thanks,
Slava.