From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1757893AbcASJk7 (ORCPT ); Tue, 19 Jan 2016 04:40:59 -0500 Received: from mx2.suse.de ([195.135.220.15]:38874 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1757379AbcASJky convert rfc822-to-8bit (ORCPT ); Tue, 19 Jan 2016 04:40:54 -0500 Date: Tue, 19 Jan 2016 10:40:51 +0100 From: Jean Delvare To: Pali =?UTF-8?B?Um9ow6Fy?= Cc: Andy Lutomirski , platform-driver-x86@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH v2 3/3] dmi: Make dmi_walk and dmi_walk_early return real error codes Message-ID: <20160119104051.21d8e2ac@endymion.delvare> In-Reply-To: <20160119090736.GF7192@pali> References: <9d1eb8634669ef09334c7e792eb21f20b320a07b.1453150613.git.luto@kernel.org> <20160119085426.7d4b1f8b@endymion.delvare> <20160119083633.GD7192@pali> <20160119100303.73cf6256@endymion.delvare> <20160119090736.GF7192@pali> Organization: SUSE Linux X-Mailer: Claws Mail 3.10.1 (GTK+ 2.24.23; x86_64-suse-linux-gnu) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8BIT Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, 19 Jan 2016 10:07:36 +0100, Pali Rohár wrote: > On Tuesday 19 January 2016 10:03:03 Jean Delvare wrote: > > Hi Pali, > > > > On Tue, 19 Jan 2016 09:36:33 +0100, Pali Rohár wrote: > > > On Tuesday 19 January 2016 08:54:26 Jean Delvare wrote: > > > > > @@ -978,11 +978,11 @@ int dmi_walk(void (*decode)(const struct dmi_header *, void *), > > > > > u8 *buf; > > > > > > > > > > if (!dmi_available) > > > > > - return -1; > > > > > + return -ENOENT; > > > > > > > > -ENOSYS would seem more appropriate? > > > > > > IIRC -ENOSYS is for non implemented syscalls. > > > > I can see a lot of -ENOSYS in include/linux/*.h returned by stubs when > > a specific subsystem is not included. Not related to syscalls at all. > > This is what lead to my suggestion. > > https://lkml.org/lkml/2014/8/22/492 Thanks for the pointer, I wasn't aware of that. It really should be documented. No, checkpatch.pl isn't documentation. Also the commit sadly doesn't say why using ENOSYS in other contexts is considered a bad thing. What actual trouble did it cause? Are the current presumably incorrect uses of ENOSYS ultimately going to be fixed? If not, I see no point in preventing other use cases. Also what about errno(3)? It says ENOSYS is "Function not implemented" - no mention to syscalls. And glibc agrees (set errno to ENOSYS and call perror, it says "Function not implemented.") In contradiction with the proposed rule. Back to the patch under review, I would argue that ENOENT is solely for file-related operations (after all the man page says "No such file or directory"), which is why I did not like it. Then what is left to mean "function not available"? ENOTSUP? Whatever the answer is, it should also be documented and added to the checkpatch warning message (if we stick to that plan...) If you tell people "don't use this" without telling them what to use instead, each of us will come up with something different and this will lead to inconsistency. Which is worse than everybody using the same (maybe suboptimal) error code. -- Jean Delvare SUSE L3 Support