From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1757166AbdELHJx (ORCPT ); Fri, 12 May 2017 03:09:53 -0400 Received: from aserp1040.oracle.com ([141.146.126.69]:20393 "EHLO aserp1040.oracle.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755728AbdELHJw (ORCPT ); Fri, 12 May 2017 03:09:52 -0400 Date: Fri, 12 May 2017 10:09:09 +0300 From: Dan Carpenter To: Stephen Hemminger Cc: SF Markus Elfring , Stephen Hemminger , Wolfram Sang , Haiyang Zhang , kernel-janitors@vger.kernel.org, LKML , devel@linuxdriverproject.org Subject: Re: [PATCH 2/4] vmbus: Delete an error message for a failed memory allocation in vmbus_device_create() Message-ID: <20170512070909.xxa62d2cde3qoj4f@mwanda> References: <587dbcf5-8b12-fac7-693e-5f471e6d5167@users.sourceforge.net> <20170511093015.4cdfdb6c@xeon-e3> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20170511093015.4cdfdb6c@xeon-e3> User-Agent: NeoMutt/20170113 (1.7.2) X-Source-IP: aserv0021.oracle.com [141.146.126.233] Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu, May 11, 2017 at 09:30:15AM -0700, Stephen Hemminger wrote: > > diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c > > index 6802d74f162c..96328aebae5a 100644 > > --- a/drivers/hv/vmbus_drv.c > > +++ b/drivers/hv/vmbus_drv.c > > @@ -1149,7 +1149,5 @@ struct hv_device *vmbus_device_create(const uuid_le *type, > > - if (!child_device_obj) { > > - pr_err("Unable to allocate device object for child device\n"); > > + if (!child_device_obj) > > return NULL; > > - } > > > > child_device_obj->channel = channel; > > memcpy(&child_device_obj->dev_type, type, sizeof(uuid_le)); > > Taking out the message assumes that all callers of this function either log an > error or pass appropriate error code back to userspace. Did you walk back > through all the callers? > > Just because an automated tool says that this needs to change does not > mean it has to. Checkpatch.pl is correct here. This message is useless. It's during init so it's unlikely to fail ever. In current kernels small kmallocs are quaranteed to succeed so it can't actually fail currently. The stack trace is more useful than this message because it tells you a lot about what memory is free and the whole call tree. The error message is dead useless code. This patch is not going to be merged because Markus doesn't listen to feedback and he's blocked but otherwise it's an OK patch. regards, dan carpenter