lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Thu, 07 Jun 2012 11:21:46 +0200 From: Stefani Seibold <stefani@...bold.net> To: Bjørn Mork <bjorn@...k.no> Cc: linux-kernel@...r.kernel.org, gregkh@...uxfoundation.org, oneukum@...e.de, linux-usb@...r.kernel.org Subject: Re: [PATCH 02/13] code cleanup Am Donnerstag, den 07.06.2012, 11:06 +0200 schrieb Bjørn Mork: > stefani@...bold.net 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. > If it is necessary... I get alway ten different complains from six developers. Developer A says do it in this way, developer B do it in the other way. > Focus on creating a *good* example. Compacting the code is not > necessarily improving the code... > Compacting improves since it will make the code more readable. > > > > /* 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? > Consistency - there are a lot places in the driver skeleton handling this in the same way. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@...r.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists