On Sun, 2010-10-24 at 14:35 -0700, Mark Brown wrote: > On Sun, Oct 24, 2010 at 08:18:59AM -0500, Timur Tabi wrote: > > On Sat, Oct 23, 2010 at 1:24 PM, Dimitris Papastamos > > > > The only problem I see with the above code, is when > > > codec_drv->reg_word_size > sizeof (unsigned int) but that can't really > > > happen in practice. > > Plus if it did happen the rest of the code would fall over fairly badly > since we've got that assumption embedded in the register read and write > APIs. > > > I'm going to have to agree with Mark that this code is suspect. I > > understand everything you said, but it makes me nervous. Unless this > > code is in some kind of fast-path, I would prefer to see it rewritten > > to avoid any assumption about the sizes of the types involved. > > I think the important thing here is that the code is clear - from a > maintainability so long as it's clear how and why the code works things > should be fine, otherwise we'll have people scratching their heads over > it every time someone looks at the code which is going to be painful. > This could be done with documentation as well as with code changes, > though code changes should definitely be considered. Yes that makes sense. The reason why I did this in the first place was to make it work with 8/16-byte reads/writes without using branching. I will change this to explicitly dereference a u8/u16 pointer. Thanks, Dimitrios _______________________________________________ Alsa-devel mailing list Alsa-devel@xxxxxxxxxxxxxxxx http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
