Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756221AbbFCUWw (ORCPT ); Wed, 3 Jun 2015 16:22:52 -0400 Received: from mail-pa0-f47.google.com ([209.85.220.47]:36246 "EHLO mail-pa0-f47.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755012AbbFCUWo (ORCPT ); Wed, 3 Jun 2015 16:22:44 -0400 Date: Thu, 4 Jun 2015 05:22:37 +0900 From: Tejun Heo To: Louis Langholtz Cc: Rusty Russell , Andrew Morton , linux-kernel@vger.kernel.org Subject: Re: kernel/params.c: 'err' variable "set but not used" and perhaps should be? Message-ID: <20150603202237.GI20091@mtj.duckdns.org> References: <87vbf628uy.fsf@rustcorp.com.au> <7E8FFE0D-B9F7-4816-8ECF-2AA5980F3890@me.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <7E8FFE0D-B9F7-4816-8ECF-2AA5980F3890@me.com> User-Agent: Mutt/1.5.23 (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2574 Lines: 58 Hello, On Wed, Jun 03, 2015 at 01:41:40PM -0600, Louis Langholtz wrote: > On Jun 1, 2015, at 7:32 PM, Rusty Russell wrote: > > That's hilarious. > > > > __attribute__((warn_unused_result)) was added to gcc as a hack so people > > wouldn't forget to use the realloc return, which probably seemed sane. > > Explains why you can't suppress it by casting to void, because for > > realloc, that would be dumb. > > > > Everyone loved it so much, they sprinkled little must-check turds > > everywhere! Because MY FUNCTION IS IMPORTANT YOU SIMPLETON, YOU MUST > > CHECK THE RETURN! > > > > The problem with yelling "YOU MUST DO SOMETHING" is that the answer is > > often "this is something, therefore it must be done". That's what > > happened here. > > The function sysfs_create_file is marked as __must_check in the > include/linux/sysfs.h file. This specific attribution appears to have been > added to this function back on September 20, 2007 by Tejun Heo. I have CC'd > Tejun so he has opportunity to respond to this criticism that you have raised. Well, it's a while ago and I don't strongly object to removing it but I still think it's a good idea to keep it around. The failure of the function creates a userland visible behavior difference and it's kinda easy to forget that it may fail as it often is the final action taken on the object. > > @@ -853,7 +853,8 @@ static void __init version_sysfs_builtin(void) > > mk = locate_module_kobject(vattr->module_name); > > if (mk) { > > - err = sysfs_create_file(&mk->kobj, &vattr->mattr.attr); > > + if (sysfs_create_file(&mk->kobj, &vattr->mattr.attr)) > > + doesnt_happen(); > > kobject_uevent(&mk->kobj, KOBJ_ADD); > > kobject_put(&mk->kobj); > > } > > Arguably then, the BUG_ON macro seems more appropriate for this situation > than this suggested doesnt_happen macro or my original offering of a call to > pr_warning. It does happen. If you don't wanna roll back on failure, just wrap it in WARN_ON() so that there's at least some indication that something failed there? It'd kinda suck to be missing some interface files w/o any indication, wouldn't it? Thanks. -- tejun -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/