On Thu, 3 Feb 2005, Fruhwirth Clemens wrote:
> First attempt:
>
> static inline int scatterwalk_needscratch(struct scatter_walk *walk, int
> nbytes) {
> return ((nbytes) <= (walk)->len_this_page &&
> (((unsigned long)(walk)->data) & (PAGE_CACHE_SIZE - 1)) +
> (nbytes) <= PAGE_CACHE_SIZE);
> }
>
> While trying to improve this unreadable monster I noticed, that
> (((unsigned long)(walk)->data) & (PAGE_CACHE_SIZE - 1)) is always equal
> to walk->offset. walk->data and walk->offset always grows together (see
> scatterwalk_copychunks), and when the bitwise AND-ing of walk->data with
> PAGE_CACHE_SIZE-1 would result walk->offset to be zero, in just that
> moment, walk->offset is set zero (see scatterwalk_pagedone). So, better:
>
> static inline int scatterwalk_needscratch(struct scatter_walk *walk, int
> nbytes)
> {
> return (nbytes <= walk->len_this_page &&
> (nbytes + walk->offset) <= PAGE_CACHE_SIZE);
> }
>
This appears to be correct. Adam (cc'd) did some work on this code, and
may have some further observations.
> Looks nicer, right? But in fact, it's redundant. walk->offset is never
> intended to grow bigger than PAGE_CACHE_SIZE, and further it's illegal
> to hand cryptoapi a scatterlist, where sg->offset is greater than
> PAGE_CACHE_SIZE (I presume this from the calculations in
> scatterwalk_start). Are these two conclusions correct, James?
Yes, passing in an offset beyond the page size is wrong.
Also, I don't know why PAGE_CACHE_SIZE is being used here instead of
PAGE_SIZE. Even though they're always the same now, I would suggest
changing all occurrences to PAGE_SIZE.
- James
--
James Morris
<[EMAIL PROTECTED]>
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
