On Sat, 2008-02-16 at 00:05 +0000, Alan Cox wrote: > On Fri, 15 Feb 2008 15:08:50 -0800 > Harvey Harrison <harvey.harrison@xxxxxxxxx> wrote: > > > On Fri, 2008-02-15 at 22:53 +0000, Alan Cox wrote: > > > > > NAK. This is a sparse bug, fix sparse. > > > > > > > > Yes, fair enough, but that's not all the patch is about. > > > > > > > > 1) it's using a max_t and min_t to force the comparisons as shorts, why > > > > not just make it a static inline? > > > > > > Because max_t and min_t also force the comparsion types > > > > Umm, maybe I'm missing something then, but how does the static inline > > not do this? > > You claimed it was an advantage of the static inline earlier but both do > anyway Oh, I thought I said it accomplished the _same_ typechecking, my bad. > > > OK, maybe not much clearer. But isn't the inline easier to see at > > a glance that it is returning a value constrained to be > > > > vmin <= v <= vmax > > > > I suppose the variable names make it clear, but the macro construction > > is (slightly) less obvious. > > Perhaps then clamp_t() > > > __mint __maxt...but I'm not proposing that. > > I am - as I bet there are other examples of that construct in the tree. Lots, but form what I've seen, most could use a helper inline that is a lot cleaner than what is currently there. This case is about the same either way. > > > > gcc still sometimes seems to optimise macros better than inlines. > > > > OK, I didn't realize that, any pointers? > > Not offhand, there is discussion in the archives but it may be somewhat > out of date for the latest gcc. > > I'm not arguing your change is -wrong- I just think the original is > tidier and clearer. Its up to Jeff anyway Fair enough. This change would make sparse a whole lot more useful for libata, as it's down to 6 warnings with this and my 3/3 patch removing another min/max nesting. Going forward this would probably make it easier on the maintainer to script up some automated checking. Cheers, Harvey - To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
