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