Hi Jean, On Sun, Feb 13, 2011 at 12:05:55PM -0500, Jean Delvare wrote: [ ... ] > > > > No good idea how to split it, though. > > Well, if nothing else, you have two changes mentioned in file CHANGES. > This would seem to be a natural split point. > > You may think it's not worth it because one of the patches will be > pretty small. However, please remember that the time it takes to review > a patch is more or less proportional to the square of its length [1]. > So even moving 10% of a patch to another may result in significant > review time savings. > > [1] This totally arbitrary and unproven statement is known as Jean > Delvare's rule ;) > I like that ;). I'll see what I can do. [ ... ] > > > > > > > > - /* check for block data */ > > > > len = 0; > > > > > > It's confusing why you initialize len here. I think it would make more > > > sense to initialize it later, where you set it for block transactions. > > > > The compiler doesn't understand that len is only used for block transactions > > in confirm(), and complains about an uninitialized variable otherwise. > > So it needs to be initialized for all commands. > > Sorry, I should have empathized my point. Once again: > > It's confusing why you initialize len _here_. > > > Let me know if you have a better idea how to handle this - I don't like it > > too much either. For now I just added a comment explaining the reason for > > the initialization. > > I get that it has to be initialized. I just don't think this is the > most logical place. > Ah, now I understand. I moved it to just before switch (size) The current location is actually a leftover from the old code. Thanks, Guenter -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
