From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753454AbbC3Rhb (ORCPT ); Mon, 30 Mar 2015 13:37:31 -0400 Received: from mout.gmx.net ([212.227.17.22]:61034 "EHLO mout.gmx.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753426AbbC3RhY (ORCPT ); Mon, 30 Mar 2015 13:37:24 -0400 Message-ID: <551989CB.20902@gmx.de> Date: Mon, 30 Mar 2015 19:37:15 +0200 From: Heinrich Schuchardt User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Icedove/31.5.0 MIME-Version: 1.0 To: Alexey Dobriyan CC: Linux Kernel , Andrew Morton Subject: Re: [PATCH 0/3] sysctl: detect overflows when setting integers References: In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-Provags-ID: V03:K0:3wOwNVWxXiUWw+inkS4yueARGYy9ZKvCdhqvbXj3pfyvR4JvnOP 3h1NrSnqvpNlPsg/CKDuTrtiYQt9FPM4n0RW3rV4kqMPu5H0VMCq9V1UeXa7Xd3IAyMJoY9 ImZuznjG7Y8ra8NZuXaQEFhKPqk4215XtNPp5SpfBZPth7GqT5WtdVG0fjniSRejalwDtjT 7qjYNyHx/fma+9l6tMSGQ== X-UI-Out-Filterresults: notjunk:1; Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hello Alexey, thank you for reviewing. On 30.03.2015 14:34, Alexey Dobriyan wrote: >> Unfortunately functions simple_strtoul and simple_strtoull cannot >> be replaced by kstrtoul and kstrtoull in some places, because they >> expect a zero terminated string instead of returning a pointer to >> the character after the last digit. >> >> This patch introduces two new functions kstrtoul_e and kstrtoull_e >> which fill this gap. > > Well, there were two ideas: > a) to convert first, see what's left and generalize it, > b) kstrtox() should be used only in one place -- > parsing integers in proc/sysfs files. Neither a) nor b) are mentioned in your patch https://lkml.org/lkml/2011/2/26/52. How does a) apply to my patch series? The patch series is about parsing integers inside the /proc mount so what is the conflict with b)? vsprintf.c has the following comment for simple_strtol: * This function is obsolete. Please use kstrtol instead. Could you, please, elaborate why kstrox.c should not be used in other places? Should we duplicate these functions instead of reusing them? > > The functions can probably be replaced by sscanf() (I didn't look closely). No. sscanf does not return the end of the parsed string. It requires to know beforehand if the string contains a octal, decimal or hexadecimal number. > > I hate "_e" suffix with passion. Which names would you appreciate? > > C in 2015 doesn't have this arcane concept known as optional > parameters so I'd suggest to hack around with always returning > number of OK characters Which functions should return the number of OK characters? > OR add one (just one) more separate > interface > > unsigned int parse_integer(const char *s, unsigned int base, T *p); Why would you call this function parse_integer and not kstrtox? > > and dispatching with __builtin_choose_expr(). https://gcc.gnu.org/onlinedocs/gcc-3.4.5/gcc/Other-Builtins.html describes how to use __builtin_types_compatible_p together with __builtin_choose_expr(). I do not understand how to avoid exposing the interfaces of the functions actually called by a parse_integer function multiplexer. > > Of course if someone rewrites that abomination called > sysctl parsing from scratch, maybe none of this will be needed! Unfortunately you are very vague about what you dislike in sysctl. When replying, please use the existing mail thread. Best regards Heinrich