Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S933207AbbFCTlv (ORCPT ); Wed, 3 Jun 2015 15:41:51 -0400 Received: from st11p01im-asmtp001.me.com ([17.172.204.151]:56668 "EHLO st11p01im-asmtp001.me.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S933054AbbFCTlo convert rfc822-to-8bit (ORCPT ); Wed, 3 Jun 2015 15:41:44 -0400 X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10432:5.14.151,1.0.33,0.0.0000 definitions=2015-06-03_10:2015-06-03,2015-06-03,1970-01-01 signatures=0 X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 spamscore=0 suspectscore=0 phishscore=0 adultscore=0 bulkscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=7.0.1-1412110000 definitions=main-1506030242 Content-type: text/plain; charset=us-ascii MIME-version: 1.0 (Mac OS X Mail 7.3 \(1878.6\)) Subject: Re: kernel/params.c: 'err' variable "set but not used" and perhaps should be? From: Louis Langholtz In-reply-to: <87vbf628uy.fsf@rustcorp.com.au> Date: Wed, 03 Jun 2015 13:41:40 -0600 Cc: Andrew Morton , linux-kernel@vger.kernel.org, htejun@gmail.com Content-transfer-encoding: 8BIT Message-id: <7E8FFE0D-B9F7-4816-8ECF-2AA5980F3890@me.com> References: <87vbf628uy.fsf@rustcorp.com.au> To: Rusty Russell X-Mailer: Apple Mail (2.1878.6) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 3288 Lines: 72 On Jun 1, 2015, at 7:32 PM, Rusty Russell wrote: > Louis Langholtz writes: >> I get a compiler warning (on compiling the linux kernel) about the 'err' >> variable being "set but not used" in the version_sysfs_builtin() function >> of kernel/params.c (at line 848). Should it be used? >> >> The 'err' variable is getting its value from the sysfs_create_file() >> function which is marked '__must_check'. If it's used, a call at least to >> printk() about any failure (indicated by a non-zero value) would seem >> useful. Here's a patch to do just that (if that alone is helpful): >> ... > > 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. Andrew Morton may have established the precedent for using __must_check in this file with his August 14, 2006 commit with the message that includes the following statements: "There's just no reason to ignore errors which can and do occur. So the patch sprinkles __must_check all over these APIs." Given this, I'd also like to hear what Andrew's thoughts are on this criticism. > ... > Instead, I suggest we introduce the following, taken literally from > various bits of userspace code I've written: > > +/* Gcc's warn_unused_result is fascist bullshit. */ > +#define doesnt_matter() > +#define doesnt_happen() > > And apply it: > > diff --git a/kernel/params.c b/kernel/params.c > index a22d6a7..fafd94a 100644 > --- a/kernel/params.c > +++ b/kernel/params.c > @@ -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. I'm curious what the LKML thinks about this issue too.-- 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/