stefani@xxxxxxxxxxx writes: > @@ -95,15 +93,12 @@ static int skel_open(struct inode *inode, struct file *file) > if (!interface) { > pr_err("%s - error, can't find device for minor %d\n", > __func__, subminor); > - retval = -ENODEV; > - goto exit; > + return -ENODEV; > } This may save you a line, but that line was there for a reason... Using a common exit path for errors makes it easier to keep unlocking, deallocation and other cleanups correct. Although you *can* do that change now, you introduce future bugs here. Someone adding a lock before this will now have to go through all the error paths to ensure that they unlock before exiting. See "Chapter 7: Centralized exiting of functions" in Documentation/CodingStyle. Most of this patch consists of this kind of bogus changes. I won't comment on the rest of them. Focus on creating a *good* example. Compacting the code is not necessarily improving the code... > /* verify that we actually have some data to write */ > - if (count == 0) > - goto exit; > + if (!count) > + return 0; zero-testing is discussed over and over again, and is a matter of taste. But I fail to see how changing it can be part of a cleanup. It just changes the flavour to suit another taste. What's the reason for doing that? Bjørn -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html