Trent Piepho <xyzzy@xxxxxxxxxxxxx> writes: > On Mon, 1 Jun 2009, Robert Jarzmik wrote: >> Trent Piepho <xyzzy@xxxxxxxxxxxxx> writes: >> > Please pull from http://linuxtv.org/hg/~tap/v4l-dvb >> If I'm not mistaken, these lines are an equivalent of : >> balign = 1 << align; >> if (align) >> x = ALIGN(x + 1 - balign/2, balign); >> >> Isn't that simpler to read ? > > I don't think so, it's not obvious that it will round to the nearest value. > You have to look up ALIGN and then __ALIGN_MASK and see that it will in > effect add balign - 1 and then reduce x + 1 - balign/2 + balign - 1 into x > + balign/2. You're thinking from your code. >> x = ALIGN(x + 1 - balign/2, balign); That means : "take x, add 1, substract half of alignment, and return nearest aligned value". IOW, by substracting half of the alignment, you have the guarantee that if you're at a distance of less half alignment from aligned value N, result will be N. > It also generates worse code. You'd think the compiled would be able to > optimize it into the same code, but it doesn't. Unless there is some issue > with how it will work with negative values that causes a subtle difference. That's a sensible argument. I presume you checked the assembly here. I'm still thinking a balign/2 is clearer than (1 << (align - 1)), but well, as it appears I'm the only one bothered, let's go ahead. What about the comment codying style comment of pxa_camera ? -- Robert -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
