On Wed, Nov 28, 2012 at 05:55:13PM +0100, Per Forlin wrote: > I have tried to work out an if-statement to check this properly. Here > is my conclusion, > This only works if sg len is 1 (in the SDIO case) > > if (WRITE) > align = sg->offset & 3 > else if (READ) > align = 0 > > if (sg->offset & 3 && (PAGE_SIZE - (sg->offset + align) < host->size)) > error; /* cannot be handled by mmci driver */ This looks wrong. For starters, why in the write case do you _add_ the byte offset in the word to sg->offset ? So, if we're offset by one byte, it becomes two bytes. Makes no sense. Secondly, (PAGE_SIZE - whatever) < host->size will be mostly always true. What that means is the check is effectively just "sg->offset & 3". Ah, maybe that's what you're trying to do - obfuscate the code in the hope that I won't analyse it so you can get your check past me and then remove all the obfuscation later... but maybe I'm just paranoid. :) > Is this if-statement align with your view of the alignment issue ? I've no idea why you're making it soo complicated. Let's review. It copes fine with aligned buffers. It copes fine with misaligned buffers that don't cross a page boundary. It also copes fine with transfers that end with a non-word sized number of bytes (which we don't need a check for). So: /* We can't cope with a misaligned transfer which crosses a page */ if (sg->offset & 3 && (sg->offset + sg->len > PAGE_SIZE || data->sg_len > 1)) bad; See what I've done there? I've taken the English and converted it directly to code. And the comment also matches the code. And this way we continue to allow buffers that the string-IO operations can cope with just fine, but avoid the issue of a word crossing a page boundary. -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
