From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-3.0 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,SPF_PASS,URIBL_BLOCKED,USER_AGENT_NEOMUTT autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id EC3D8C04E53 for ; Wed, 15 May 2019 07:35:47 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id C7D1C20862 for ; Wed, 15 May 2019 07:35:47 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726406AbfEOHfr (ORCPT ); Wed, 15 May 2019 03:35:47 -0400 Received: from mx2.suse.de ([195.135.220.15]:54248 "EHLO mx1.suse.de" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1725912AbfEOHfq (ORCPT ); Wed, 15 May 2019 03:35:46 -0400 X-Virus-Scanned: by amavisd-new at test-mx.suse.de Received: from relay2.suse.de (unknown [195.135.220.254]) by mx1.suse.de (Postfix) with ESMTP id 8E79AABE9; Wed, 15 May 2019 07:35:44 +0000 (UTC) Date: Wed, 15 May 2019 09:35:42 +0200 From: Petr Mladek To: Steven Rostedt Cc: Geert Uytterhoeven , David Laight , Sergey Senozhatsky , Andy Shevchenko , christophe leroy , Linus Torvalds , Rasmus Villemoes , "Tobin C . Harding" , Michal Hocko , Sergey Senozhatsky , "linux-kernel@vger.kernel.org" , Michael Ellerman , "linuxppc-dev@lists.ozlabs.org" , Russell Currey , Stephen Rothwell , Heiko Carstens , "linux-arch@vger.kernel.org" , "linux-s390@vger.kernel.org" , Martin Schwidefsky Subject: Re: [PATCH] vsprintf: Do not break early boot with probing addresses Message-ID: <20190515073542.y6ru2nfagtcrpdl7@pathway.suse.cz> References: <20190510084213.22149-1-pmladek@suse.com> <20190510122401.21a598f6@gandalf.local.home> <096d6c9c17b3484484d9d9d3f3aa3a7c@AcuMS.aculab.com> <20190513091320.GK9224@smile.fi.intel.com> <20190513124220.wty2qbnz4wo52h3x@pathway.suse.cz> <20190514020730.GA651@jagdpanzerIV> <45348cf615fe40d383c1a25688d4a88f@AcuMS.aculab.com> <20190514143751.48e81e05@oasis.local.home> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20190514143751.48e81e05@oasis.local.home> User-Agent: NeoMutt/20170912 (1.9.0) Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue 2019-05-14 14:37:51, Steven Rostedt wrote: > > [ Purple is a nice shade on the bike shed. ;-) ] > > On Tue, 14 May 2019 11:02:17 +0200 > Geert Uytterhoeven wrote: > > > On Tue, May 14, 2019 at 10:29 AM David Laight wrote: > > > > And I like Steven's "(fault)" idea. > > > > How about this: > > > > > > > > if ptr < PAGE_SIZE -> "(null)" > > > > if IS_ERR_VALUE(ptr) -> "(fault)" > > > > > > > > -ss > > > > > > Or: > > > if (ptr < PAGE_SIZE) > > > return ptr ? "(null+)" : "(null)"; > > Hmm, that is useful. > > > > if IS_ERR_VALUE(ptr) > > > return "(errno)" > > I still prefer "(fault)" as is pretty much all I would expect from a > pointer dereference, even if it is just bad parsing of, say, a parsing > an MAC address. "fault" is generic enough. "errno" will be confusing, > because that's normally a variable not a output. > > > > > Do we care about the value? "(-E%u)"? > > That too could be confusing. What would (-E22) be considered by a user > doing an sprintf() on some string. I know that would confuse me, or I > would think that it was what the %pX displayed, and wonder why it > displayed it that way. Whereas "(fault)" is quite obvious for any %p > use case. This discussion clearly shows that it is hard to make anyone happy. I considered switching to "(fault)" because there seems to be more people in favor of this. But there is used also "(einval)" when an unsupported pointer modifier is passed. The idea is to show error codes that people are familiar with. It might have been better to use the uppercase "(EFAULT)" and "(EINVAL)" to make it more obvious. But I wanted to follow the existing style with the lowercase "(null)". As of now, I think that we should keep it as is unless there is some wider agreement on a change. Best Regards, Petr From mboxrd@z Thu Jan 1 00:00:00 1970 From: Petr Mladek Subject: Re: [PATCH] vsprintf: Do not break early boot with probing addresses Date: Wed, 15 May 2019 09:35:42 +0200 Message-ID: <20190515073542.y6ru2nfagtcrpdl7@pathway.suse.cz> References: <20190510084213.22149-1-pmladek@suse.com> <20190510122401.21a598f6@gandalf.local.home> <096d6c9c17b3484484d9d9d3f3aa3a7c@AcuMS.aculab.com> <20190513091320.GK9224@smile.fi.intel.com> <20190513124220.wty2qbnz4wo52h3x@pathway.suse.cz> <20190514020730.GA651@jagdpanzerIV> <45348cf615fe40d383c1a25688d4a88f@AcuMS.aculab.com> <20190514143751.48e81e05@oasis.local.home> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Return-path: Content-Disposition: inline In-Reply-To: <20190514143751.48e81e05@oasis.local.home> Sender: linux-kernel-owner@vger.kernel.org To: Steven Rostedt Cc: Geert Uytterhoeven , David Laight , Sergey Senozhatsky , Andy Shevchenko , christophe leroy , Linus Torvalds , Rasmus Villemoes , "Tobin C . Harding" , Michal Hocko , Sergey Senozhatsky , "linux-kernel@vger.kernel.org" , Michael Ellerman , "linuxppc-dev@lists.ozlabs.org" , Russell Currey , Stephen Rothwell , Heiko Carstens , "linux-arch@vger.kernel.org" List-Id: linux-arch.vger.kernel.org On Tue 2019-05-14 14:37:51, Steven Rostedt wrote: > > [ Purple is a nice shade on the bike shed. ;-) ] > > On Tue, 14 May 2019 11:02:17 +0200 > Geert Uytterhoeven wrote: > > > On Tue, May 14, 2019 at 10:29 AM David Laight wrote: > > > > And I like Steven's "(fault)" idea. > > > > How about this: > > > > > > > > if ptr < PAGE_SIZE -> "(null)" > > > > if IS_ERR_VALUE(ptr) -> "(fault)" > > > > > > > > -ss > > > > > > Or: > > > if (ptr < PAGE_SIZE) > > > return ptr ? "(null+)" : "(null)"; > > Hmm, that is useful. > > > > if IS_ERR_VALUE(ptr) > > > return "(errno)" > > I still prefer "(fault)" as is pretty much all I would expect from a > pointer dereference, even if it is just bad parsing of, say, a parsing > an MAC address. "fault" is generic enough. "errno" will be confusing, > because that's normally a variable not a output. > > > > > Do we care about the value? "(-E%u)"? > > That too could be confusing. What would (-E22) be considered by a user > doing an sprintf() on some string. I know that would confuse me, or I > would think that it was what the %pX displayed, and wonder why it > displayed it that way. Whereas "(fault)" is quite obvious for any %p > use case. This discussion clearly shows that it is hard to make anyone happy. I considered switching to "(fault)" because there seems to be more people in favor of this. But there is used also "(einval)" when an unsupported pointer modifier is passed. The idea is to show error codes that people are familiar with. It might have been better to use the uppercase "(EFAULT)" and "(EINVAL)" to make it more obvious. But I wanted to follow the existing style with the lowercase "(null)". As of now, I think that we should keep it as is unless there is some wider agreement on a change. Best Regards, Petr From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-3.0 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,SPF_PASS,URIBL_BLOCKED,USER_AGENT_NEOMUTT autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 2A6D8C04E53 for ; Wed, 15 May 2019 07:37:43 +0000 (UTC) Received: from lists.ozlabs.org (lists.ozlabs.org [203.11.71.2]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id ABA0A20881 for ; Wed, 15 May 2019 07:37:42 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org ABA0A20881 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=suse.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=linuxppc-dev-bounces+linuxppc-dev=archiver.kernel.org@lists.ozlabs.org Received: from lists.ozlabs.org (lists.ozlabs.org [IPv6:2401:3900:2:1::3]) by lists.ozlabs.org (Postfix) with ESMTP id 453mc23F6PzDqQc for ; Wed, 15 May 2019 17:37:38 +1000 (AEST) Authentication-Results: lists.ozlabs.org; spf=pass (mailfrom) smtp.mailfrom=suse.com (client-ip=195.135.220.15; helo=mx1.suse.de; envelope-from=pmladek@suse.com; receiver=) Authentication-Results: lists.ozlabs.org; dmarc=none (p=none dis=none) header.from=suse.com Received: from mx1.suse.de (mx2.suse.de [195.135.220.15]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by lists.ozlabs.org (Postfix) with ESMTPS id 453mYz12YVzDqQ5 for ; Wed, 15 May 2019 17:35:48 +1000 (AEST) X-Virus-Scanned: by amavisd-new at test-mx.suse.de Received: from relay2.suse.de (unknown [195.135.220.254]) by mx1.suse.de (Postfix) with ESMTP id 8E79AABE9; Wed, 15 May 2019 07:35:44 +0000 (UTC) Date: Wed, 15 May 2019 09:35:42 +0200 From: Petr Mladek To: Steven Rostedt Subject: Re: [PATCH] vsprintf: Do not break early boot with probing addresses Message-ID: <20190515073542.y6ru2nfagtcrpdl7@pathway.suse.cz> References: <20190510084213.22149-1-pmladek@suse.com> <20190510122401.21a598f6@gandalf.local.home> <096d6c9c17b3484484d9d9d3f3aa3a7c@AcuMS.aculab.com> <20190513091320.GK9224@smile.fi.intel.com> <20190513124220.wty2qbnz4wo52h3x@pathway.suse.cz> <20190514020730.GA651@jagdpanzerIV> <45348cf615fe40d383c1a25688d4a88f@AcuMS.aculab.com> <20190514143751.48e81e05@oasis.local.home> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20190514143751.48e81e05@oasis.local.home> User-Agent: NeoMutt/20170912 (1.9.0) X-BeenThere: linuxppc-dev@lists.ozlabs.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Linux on PowerPC Developers Mail List List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: "linux-arch@vger.kernel.org" , Sergey Senozhatsky , Heiko Carstens , "linux-s390@vger.kernel.org" , "linuxppc-dev@lists.ozlabs.org" , Rasmus Villemoes , "linux-kernel@vger.kernel.org" , Michal Hocko , Sergey Senozhatsky , David Laight , Geert Uytterhoeven , Stephen Rothwell , Andy Shevchenko , Linus Torvalds , Martin Schwidefsky , "Tobin C . Harding" Errors-To: linuxppc-dev-bounces+linuxppc-dev=archiver.kernel.org@lists.ozlabs.org Sender: "Linuxppc-dev" On Tue 2019-05-14 14:37:51, Steven Rostedt wrote: > > [ Purple is a nice shade on the bike shed. ;-) ] > > On Tue, 14 May 2019 11:02:17 +0200 > Geert Uytterhoeven wrote: > > > On Tue, May 14, 2019 at 10:29 AM David Laight wrote: > > > > And I like Steven's "(fault)" idea. > > > > How about this: > > > > > > > > if ptr < PAGE_SIZE -> "(null)" > > > > if IS_ERR_VALUE(ptr) -> "(fault)" > > > > > > > > -ss > > > > > > Or: > > > if (ptr < PAGE_SIZE) > > > return ptr ? "(null+)" : "(null)"; > > Hmm, that is useful. > > > > if IS_ERR_VALUE(ptr) > > > return "(errno)" > > I still prefer "(fault)" as is pretty much all I would expect from a > pointer dereference, even if it is just bad parsing of, say, a parsing > an MAC address. "fault" is generic enough. "errno" will be confusing, > because that's normally a variable not a output. > > > > > Do we care about the value? "(-E%u)"? > > That too could be confusing. What would (-E22) be considered by a user > doing an sprintf() on some string. I know that would confuse me, or I > would think that it was what the %pX displayed, and wonder why it > displayed it that way. Whereas "(fault)" is quite obvious for any %p > use case. This discussion clearly shows that it is hard to make anyone happy. I considered switching to "(fault)" because there seems to be more people in favor of this. But there is used also "(einval)" when an unsupported pointer modifier is passed. The idea is to show error codes that people are familiar with. It might have been better to use the uppercase "(EFAULT)" and "(EINVAL)" to make it more obvious. But I wanted to follow the existing style with the lowercase "(null)". As of now, I think that we should keep it as is unless there is some wider agreement on a change. Best Regards, Petr