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 Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 27001C433F5 for ; Wed, 19 Jan 2022 13:18:14 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1354756AbiASNSM (ORCPT ); Wed, 19 Jan 2022 08:18:12 -0500 Received: from smtp-out1.suse.de ([195.135.220.28]:34898 "EHLO smtp-out1.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1354779AbiASNSF (ORCPT ); Wed, 19 Jan 2022 08:18:05 -0500 Received: from relay2.suse.de (relay2.suse.de [149.44.160.134]) by smtp-out1.suse.de (Postfix) with ESMTP id 54C8621123; Wed, 19 Jan 2022 13:18:03 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.com; s=susede1; t=1642598283; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=5030rwRdQVdGOVRRFHRTkRKqifOJXNno9J3Z4c5+1YA=; b=X+sTq95MaR4ipOc0sga1XDfWcMiW9FTvop0fDJWvb/D9s4D7IGDBilB2aVkSGlh5aFcJD/ n/WEnrXPiJsFn6TQAANEUr68P561Vum2hTmCOBIaTd3It9Hqa5/LYx0lz0+wL5aAgSaYNR fPvU2djzpoRxXztcSp7BAsmfXILijxY= Received: from suse.cz (unknown [10.100.224.162]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by relay2.suse.de (Postfix) with ESMTPS id DB985A3B89; Wed, 19 Jan 2022 13:18:01 +0000 (UTC) Date: Wed, 19 Jan 2022 14:18:01 +0100 From: Petr Mladek To: Lucas De Marchi Cc: linux-kernel@vger.kernel.org, intel-gfx@lists.freedesktop.org, dri-devel@lists.freedesktop.org, amd-gfx@lists.freedesktop.org, linux-security-module@vger.kernel.org, nouveau@lists.freedesktop.org, netdev@vger.kernel.org, Alex Deucher , Andrew Morton , Andy Shevchenko , Andy Shevchenko , Ben Skeggs , Christian =?iso-8859-1?Q?K=F6nig?= , Chris Wilson , Daniel Vetter , David Airlie , "David S . Miller" , Emma Anholt , Eryk Brol , Francis Laniel , Greg Kroah-Hartman , Harry Wentland , Jakub Kicinski , Jani Nikula , Joonas Lahtinen , Julia Lawall , Kentaro Takeda , Leo Li , Mikita Lipski , Rahul Lakkireddy , Raju Rangoju , Rasmus Villemoes , Rodrigo Vivi , Sakari Ailus , Sergey Senozhatsky , Steven Rostedt , Vishal Kulkarni Subject: Re: [PATCH 0/3] lib/string_helpers: Add a few string helpers Message-ID: References: <20220119072450.2890107-1-lucas.demarchi@intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <20220119072450.2890107-1-lucas.demarchi@intel.com> Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue 2022-01-18 23:24:47, Lucas De Marchi wrote: > Add some helpers under lib/string_helpers.h so they can be used > throughout the kernel. When I started doing this there were 2 other > previous attempts I know of, not counting the iterations each of them > had: > > 1) https://lore.kernel.org/all/20191023131308.9420-1-jani.nikula@intel.com/ > 2) https://lore.kernel.org/all/20210215142137.64476-1-andriy.shevchenko@linux.intel.com/#t > > Going through the comments I tried to find some common ground and > justification for what is in here, addressing some of the concerns > raised. > > d. This doesn't bring onoff() helper as there are some places in the > kernel with onoff as variable - another name is probably needed for > this function in order not to shadow the variable, or those variables > could be renamed. Or if people wanting > try to find a short one I would call it str_on_off(). And I would actually suggest to use the same style also for the other helpers. The "str_" prefix would make it clear that it is something with string. There are other _on_off() that affect some functionality, e.g. mute_led_on_off(), e1000_vlan_filter_on_off(). The dash '_' would significantly help to parse the name. yesno() and onoff() are nicely short and kind of acceptable. But "enabledisable()" is a puzzle. IMHO, str_yes_no(), str_on_off(), str_enable_disable() are a good compromise. The main motivation should be code readability. You write the code once. But many people will read it many times. Open coding is sometimes better than misleading macro names. That said, I do not want to block this patchset. If others like it... ;-) > e. One alternative to all of this suggested by Christian König > (43456ba7-c372-84cc-4949-dcb817188e21@amd.com) would be to add a > printk format. But besides the comment, he also seemed to like > the common function. This brought the argument from others that the > simple yesno()/enabledisable() already used in the code is easier to > remember and use than e.g. %py[DOY] Thanks for not going this way :-) > Last patch also has some additional conversion of open coded cases. I > preferred starting with drm/ since this is "closer to home". > > I hope this is a good summary of the previous attempts and a way we can > move forward. > > Andrew Morton, Petr Mladek, Andy Shevchenko: if this is accepted, my > proposal is to take first 2 patches either through mm tree or maybe > vsprintf. Last patch can be taken later through drm. I agree with Andy that it should go via drm tree. It would make it easier to handle potential conflicts. Just in case, you decide to go with str_yes_no() or something similar. Mass changes are typically done at the end on the merge window. The best solution is when it can be done by a script. 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 Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 22AFCC433F5 for ; Wed, 19 Jan 2022 13:18:07 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id E66E310E13A; Wed, 19 Jan 2022 13:18:05 +0000 (UTC) Received: from smtp-out1.suse.de (smtp-out1.suse.de [195.135.220.28]) by gabe.freedesktop.org (Postfix) with ESMTPS id 0A0CD10E13A; Wed, 19 Jan 2022 13:18:05 +0000 (UTC) Received: from relay2.suse.de (relay2.suse.de [149.44.160.134]) by smtp-out1.suse.de (Postfix) with ESMTP id 54C8621123; Wed, 19 Jan 2022 13:18:03 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.com; s=susede1; t=1642598283; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=5030rwRdQVdGOVRRFHRTkRKqifOJXNno9J3Z4c5+1YA=; b=X+sTq95MaR4ipOc0sga1XDfWcMiW9FTvop0fDJWvb/D9s4D7IGDBilB2aVkSGlh5aFcJD/ n/WEnrXPiJsFn6TQAANEUr68P561Vum2hTmCOBIaTd3It9Hqa5/LYx0lz0+wL5aAgSaYNR fPvU2djzpoRxXztcSp7BAsmfXILijxY= Received: from suse.cz (unknown [10.100.224.162]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by relay2.suse.de (Postfix) with ESMTPS id DB985A3B89; Wed, 19 Jan 2022 13:18:01 +0000 (UTC) Date: Wed, 19 Jan 2022 14:18:01 +0100 From: Petr Mladek To: Lucas De Marchi Subject: Re: [PATCH 0/3] lib/string_helpers: Add a few string helpers Message-ID: References: <20220119072450.2890107-1-lucas.demarchi@intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <20220119072450.2890107-1-lucas.demarchi@intel.com> X-BeenThere: dri-devel@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Direct Rendering Infrastructure - Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Emma Anholt , David Airlie , nouveau@lists.freedesktop.org, Rasmus Villemoes , dri-devel@lists.freedesktop.org, Chris Wilson , Vishal Kulkarni , Francis Laniel , Kentaro Takeda , Mikita Lipski , amd-gfx@lists.freedesktop.org, Andy Shevchenko , Ben Skeggs , Jakub Kicinski , Sakari Ailus , Leo Li , intel-gfx@lists.freedesktop.org, Raju Rangoju , Julia Lawall , Rahul Lakkireddy , Steven Rostedt , Rodrigo Vivi , Andy Shevchenko , Eryk Brol , Greg Kroah-Hartman , linux-kernel@vger.kernel.org, Christian =?iso-8859-1?Q?K=F6nig?= , Sergey Senozhatsky , linux-security-module@vger.kernel.org, netdev@vger.kernel.org, Alex Deucher , Andrew Morton , "David S . Miller" Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" On Tue 2022-01-18 23:24:47, Lucas De Marchi wrote: > Add some helpers under lib/string_helpers.h so they can be used > throughout the kernel. When I started doing this there were 2 other > previous attempts I know of, not counting the iterations each of them > had: > > 1) https://lore.kernel.org/all/20191023131308.9420-1-jani.nikula@intel.com/ > 2) https://lore.kernel.org/all/20210215142137.64476-1-andriy.shevchenko@linux.intel.com/#t > > Going through the comments I tried to find some common ground and > justification for what is in here, addressing some of the concerns > raised. > > d. This doesn't bring onoff() helper as there are some places in the > kernel with onoff as variable - another name is probably needed for > this function in order not to shadow the variable, or those variables > could be renamed. Or if people wanting > try to find a short one I would call it str_on_off(). And I would actually suggest to use the same style also for the other helpers. The "str_" prefix would make it clear that it is something with string. There are other _on_off() that affect some functionality, e.g. mute_led_on_off(), e1000_vlan_filter_on_off(). The dash '_' would significantly help to parse the name. yesno() and onoff() are nicely short and kind of acceptable. But "enabledisable()" is a puzzle. IMHO, str_yes_no(), str_on_off(), str_enable_disable() are a good compromise. The main motivation should be code readability. You write the code once. But many people will read it many times. Open coding is sometimes better than misleading macro names. That said, I do not want to block this patchset. If others like it... ;-) > e. One alternative to all of this suggested by Christian König > (43456ba7-c372-84cc-4949-dcb817188e21@amd.com) would be to add a > printk format. But besides the comment, he also seemed to like > the common function. This brought the argument from others that the > simple yesno()/enabledisable() already used in the code is easier to > remember and use than e.g. %py[DOY] Thanks for not going this way :-) > Last patch also has some additional conversion of open coded cases. I > preferred starting with drm/ since this is "closer to home". > > I hope this is a good summary of the previous attempts and a way we can > move forward. > > Andrew Morton, Petr Mladek, Andy Shevchenko: if this is accepted, my > proposal is to take first 2 patches either through mm tree or maybe > vsprintf. Last patch can be taken later through drm. I agree with Andy that it should go via drm tree. It would make it easier to handle potential conflicts. Just in case, you decide to go with str_yes_no() or something similar. Mass changes are typically done at the end on the merge window. The best solution is when it can be done by a script. 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 Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 18D4FC433F5 for ; Wed, 19 Jan 2022 13:18:12 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id F3D8310E13C; Wed, 19 Jan 2022 13:18:05 +0000 (UTC) Received: from smtp-out1.suse.de (smtp-out1.suse.de [195.135.220.28]) by gabe.freedesktop.org (Postfix) with ESMTPS id 0A0CD10E13A; Wed, 19 Jan 2022 13:18:05 +0000 (UTC) Received: from relay2.suse.de (relay2.suse.de [149.44.160.134]) by smtp-out1.suse.de (Postfix) with ESMTP id 54C8621123; Wed, 19 Jan 2022 13:18:03 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.com; s=susede1; t=1642598283; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=5030rwRdQVdGOVRRFHRTkRKqifOJXNno9J3Z4c5+1YA=; b=X+sTq95MaR4ipOc0sga1XDfWcMiW9FTvop0fDJWvb/D9s4D7IGDBilB2aVkSGlh5aFcJD/ n/WEnrXPiJsFn6TQAANEUr68P561Vum2hTmCOBIaTd3It9Hqa5/LYx0lz0+wL5aAgSaYNR fPvU2djzpoRxXztcSp7BAsmfXILijxY= Received: from suse.cz (unknown [10.100.224.162]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by relay2.suse.de (Postfix) with ESMTPS id DB985A3B89; Wed, 19 Jan 2022 13:18:01 +0000 (UTC) Date: Wed, 19 Jan 2022 14:18:01 +0100 From: Petr Mladek To: Lucas De Marchi Message-ID: References: <20220119072450.2890107-1-lucas.demarchi@intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <20220119072450.2890107-1-lucas.demarchi@intel.com> Subject: Re: [Intel-gfx] [PATCH 0/3] lib/string_helpers: Add a few string helpers X-BeenThere: intel-gfx@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Intel graphics driver community testing & development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Emma Anholt , David Airlie , nouveau@lists.freedesktop.org, Rasmus Villemoes , dri-devel@lists.freedesktop.org, Chris Wilson , Vishal Kulkarni , Francis Laniel , Kentaro Takeda , Mikita Lipski , amd-gfx@lists.freedesktop.org, Andy Shevchenko , Ben Skeggs , Jakub Kicinski , Harry Wentland , Sakari Ailus , Leo Li , intel-gfx@lists.freedesktop.org, Raju Rangoju , Julia Lawall , Rahul Lakkireddy , Steven Rostedt , Andy Shevchenko , Eryk Brol , Greg Kroah-Hartman , linux-kernel@vger.kernel.org, Christian =?iso-8859-1?Q?K=F6nig?= , Sergey Senozhatsky , linux-security-module@vger.kernel.org, netdev@vger.kernel.org, Alex Deucher , Andrew Morton , "David S . Miller" Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" On Tue 2022-01-18 23:24:47, Lucas De Marchi wrote: > Add some helpers under lib/string_helpers.h so they can be used > throughout the kernel. When I started doing this there were 2 other > previous attempts I know of, not counting the iterations each of them > had: > > 1) https://lore.kernel.org/all/20191023131308.9420-1-jani.nikula@intel.com/ > 2) https://lore.kernel.org/all/20210215142137.64476-1-andriy.shevchenko@linux.intel.com/#t > > Going through the comments I tried to find some common ground and > justification for what is in here, addressing some of the concerns > raised. > > d. This doesn't bring onoff() helper as there are some places in the > kernel with onoff as variable - another name is probably needed for > this function in order not to shadow the variable, or those variables > could be renamed. Or if people wanting > try to find a short one I would call it str_on_off(). And I would actually suggest to use the same style also for the other helpers. The "str_" prefix would make it clear that it is something with string. There are other _on_off() that affect some functionality, e.g. mute_led_on_off(), e1000_vlan_filter_on_off(). The dash '_' would significantly help to parse the name. yesno() and onoff() are nicely short and kind of acceptable. But "enabledisable()" is a puzzle. IMHO, str_yes_no(), str_on_off(), str_enable_disable() are a good compromise. The main motivation should be code readability. You write the code once. But many people will read it many times. Open coding is sometimes better than misleading macro names. That said, I do not want to block this patchset. If others like it... ;-) > e. One alternative to all of this suggested by Christian König > (43456ba7-c372-84cc-4949-dcb817188e21@amd.com) would be to add a > printk format. But besides the comment, he also seemed to like > the common function. This brought the argument from others that the > simple yesno()/enabledisable() already used in the code is easier to > remember and use than e.g. %py[DOY] Thanks for not going this way :-) > Last patch also has some additional conversion of open coded cases. I > preferred starting with drm/ since this is "closer to home". > > I hope this is a good summary of the previous attempts and a way we can > move forward. > > Andrew Morton, Petr Mladek, Andy Shevchenko: if this is accepted, my > proposal is to take first 2 patches either through mm tree or maybe > vsprintf. Last patch can be taken later through drm. I agree with Andy that it should go via drm tree. It would make it easier to handle potential conflicts. Just in case, you decide to go with str_yes_no() or something similar. Mass changes are typically done at the end on the merge window. The best solution is when it can be done by a script. 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 Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 79983C433EF for ; Wed, 19 Jan 2022 14:32:47 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 954E910EB57; Wed, 19 Jan 2022 14:32:46 +0000 (UTC) Received: from smtp-out1.suse.de (smtp-out1.suse.de [195.135.220.28]) by gabe.freedesktop.org (Postfix) with ESMTPS id 0A0CD10E13A; Wed, 19 Jan 2022 13:18:05 +0000 (UTC) Received: from relay2.suse.de (relay2.suse.de [149.44.160.134]) by smtp-out1.suse.de (Postfix) with ESMTP id 54C8621123; Wed, 19 Jan 2022 13:18:03 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.com; s=susede1; t=1642598283; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=5030rwRdQVdGOVRRFHRTkRKqifOJXNno9J3Z4c5+1YA=; b=X+sTq95MaR4ipOc0sga1XDfWcMiW9FTvop0fDJWvb/D9s4D7IGDBilB2aVkSGlh5aFcJD/ n/WEnrXPiJsFn6TQAANEUr68P561Vum2hTmCOBIaTd3It9Hqa5/LYx0lz0+wL5aAgSaYNR fPvU2djzpoRxXztcSp7BAsmfXILijxY= Received: from suse.cz (unknown [10.100.224.162]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by relay2.suse.de (Postfix) with ESMTPS id DB985A3B89; Wed, 19 Jan 2022 13:18:01 +0000 (UTC) Date: Wed, 19 Jan 2022 14:18:01 +0100 From: Petr Mladek To: Lucas De Marchi Subject: Re: [PATCH 0/3] lib/string_helpers: Add a few string helpers Message-ID: References: <20220119072450.2890107-1-lucas.demarchi@intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <20220119072450.2890107-1-lucas.demarchi@intel.com> X-Mailman-Approved-At: Wed, 19 Jan 2022 14:32:45 +0000 X-BeenThere: amd-gfx@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Discussion list for AMD gfx List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Emma Anholt , David Airlie , nouveau@lists.freedesktop.org, Joonas Lahtinen , Rasmus Villemoes , dri-devel@lists.freedesktop.org, Chris Wilson , Vishal Kulkarni , Francis Laniel , Kentaro Takeda , Mikita Lipski , amd-gfx@lists.freedesktop.org, Andy Shevchenko , Ben Skeggs , Jakub Kicinski , Harry Wentland , Sakari Ailus , Leo Li , intel-gfx@lists.freedesktop.org, Raju Rangoju , Jani Nikula , Julia Lawall , Rahul Lakkireddy , Steven Rostedt , Rodrigo Vivi , Andy Shevchenko , Eryk Brol , Greg Kroah-Hartman , linux-kernel@vger.kernel.org, Christian =?iso-8859-1?Q?K=F6nig?= , Sergey Senozhatsky , linux-security-module@vger.kernel.org, Daniel Vetter , netdev@vger.kernel.org, Alex Deucher , Andrew Morton , "David S . Miller" Errors-To: amd-gfx-bounces@lists.freedesktop.org Sender: "amd-gfx" On Tue 2022-01-18 23:24:47, Lucas De Marchi wrote: > Add some helpers under lib/string_helpers.h so they can be used > throughout the kernel. When I started doing this there were 2 other > previous attempts I know of, not counting the iterations each of them > had: > > 1) https://lore.kernel.org/all/20191023131308.9420-1-jani.nikula@intel.com/ > 2) https://lore.kernel.org/all/20210215142137.64476-1-andriy.shevchenko@linux.intel.com/#t > > Going through the comments I tried to find some common ground and > justification for what is in here, addressing some of the concerns > raised. > > d. This doesn't bring onoff() helper as there are some places in the > kernel with onoff as variable - another name is probably needed for > this function in order not to shadow the variable, or those variables > could be renamed. Or if people wanting > try to find a short one I would call it str_on_off(). And I would actually suggest to use the same style also for the other helpers. The "str_" prefix would make it clear that it is something with string. There are other _on_off() that affect some functionality, e.g. mute_led_on_off(), e1000_vlan_filter_on_off(). The dash '_' would significantly help to parse the name. yesno() and onoff() are nicely short and kind of acceptable. But "enabledisable()" is a puzzle. IMHO, str_yes_no(), str_on_off(), str_enable_disable() are a good compromise. The main motivation should be code readability. You write the code once. But many people will read it many times. Open coding is sometimes better than misleading macro names. That said, I do not want to block this patchset. If others like it... ;-) > e. One alternative to all of this suggested by Christian König > (43456ba7-c372-84cc-4949-dcb817188e21@amd.com) would be to add a > printk format. But besides the comment, he also seemed to like > the common function. This brought the argument from others that the > simple yesno()/enabledisable() already used in the code is easier to > remember and use than e.g. %py[DOY] Thanks for not going this way :-) > Last patch also has some additional conversion of open coded cases. I > preferred starting with drm/ since this is "closer to home". > > I hope this is a good summary of the previous attempts and a way we can > move forward. > > Andrew Morton, Petr Mladek, Andy Shevchenko: if this is accepted, my > proposal is to take first 2 patches either through mm tree or maybe > vsprintf. Last patch can be taken later through drm. I agree with Andy that it should go via drm tree. It would make it easier to handle potential conflicts. Just in case, you decide to go with str_yes_no() or something similar. Mass changes are typically done at the end on the merge window. The best solution is when it can be done by a script. 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 Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 5352BC433FE for ; Thu, 20 Jan 2022 18:17:01 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id B40BE10E658; Thu, 20 Jan 2022 18:16:52 +0000 (UTC) Received: from smtp-out1.suse.de (smtp-out1.suse.de [195.135.220.28]) by gabe.freedesktop.org (Postfix) with ESMTPS id 0A0CD10E13A; Wed, 19 Jan 2022 13:18:05 +0000 (UTC) Received: from relay2.suse.de (relay2.suse.de [149.44.160.134]) by smtp-out1.suse.de (Postfix) with ESMTP id 54C8621123; Wed, 19 Jan 2022 13:18:03 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.com; s=susede1; t=1642598283; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=5030rwRdQVdGOVRRFHRTkRKqifOJXNno9J3Z4c5+1YA=; b=X+sTq95MaR4ipOc0sga1XDfWcMiW9FTvop0fDJWvb/D9s4D7IGDBilB2aVkSGlh5aFcJD/ n/WEnrXPiJsFn6TQAANEUr68P561Vum2hTmCOBIaTd3It9Hqa5/LYx0lz0+wL5aAgSaYNR fPvU2djzpoRxXztcSp7BAsmfXILijxY= Received: from suse.cz (unknown [10.100.224.162]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by relay2.suse.de (Postfix) with ESMTPS id DB985A3B89; Wed, 19 Jan 2022 13:18:01 +0000 (UTC) Date: Wed, 19 Jan 2022 14:18:01 +0100 From: Petr Mladek To: Lucas De Marchi Message-ID: References: <20220119072450.2890107-1-lucas.demarchi@intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <20220119072450.2890107-1-lucas.demarchi@intel.com> X-Mailman-Approved-At: Thu, 20 Jan 2022 18:16:51 +0000 Subject: Re: [Nouveau] [PATCH 0/3] lib/string_helpers: Add a few string helpers X-BeenThere: nouveau@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Nouveau development list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Emma Anholt , David Airlie , nouveau@lists.freedesktop.org, Joonas Lahtinen , Rasmus Villemoes , dri-devel@lists.freedesktop.org, Chris Wilson , Vishal Kulkarni , Francis Laniel , Kentaro Takeda , Mikita Lipski , amd-gfx@lists.freedesktop.org, Andy Shevchenko , Ben Skeggs , Jakub Kicinski , Harry Wentland , Sakari Ailus , Leo Li , intel-gfx@lists.freedesktop.org, Raju Rangoju , Jani Nikula , Julia Lawall , Rahul Lakkireddy , Steven Rostedt , Rodrigo Vivi , Andy Shevchenko , Eryk Brol , Greg Kroah-Hartman , linux-kernel@vger.kernel.org, Christian =?iso-8859-1?Q?K=F6nig?= , Sergey Senozhatsky , linux-security-module@vger.kernel.org, Daniel Vetter , netdev@vger.kernel.org, Alex Deucher , Andrew Morton , "David S . Miller" Errors-To: nouveau-bounces@lists.freedesktop.org Sender: "Nouveau" On Tue 2022-01-18 23:24:47, Lucas De Marchi wrote: > Add some helpers under lib/string_helpers.h so they can be used > throughout the kernel. When I started doing this there were 2 other > previous attempts I know of, not counting the iterations each of them > had: > > 1) https://lore.kernel.org/all/20191023131308.9420-1-jani.nikula@intel.com/ > 2) https://lore.kernel.org/all/20210215142137.64476-1-andriy.shevchenko@linux.intel.com/#t > > Going through the comments I tried to find some common ground and > justification for what is in here, addressing some of the concerns > raised. > > d. This doesn't bring onoff() helper as there are some places in the > kernel with onoff as variable - another name is probably needed for > this function in order not to shadow the variable, or those variables > could be renamed. Or if people wanting > try to find a short one I would call it str_on_off(). And I would actually suggest to use the same style also for the other helpers. The "str_" prefix would make it clear that it is something with string. There are other _on_off() that affect some functionality, e.g. mute_led_on_off(), e1000_vlan_filter_on_off(). The dash '_' would significantly help to parse the name. yesno() and onoff() are nicely short and kind of acceptable. But "enabledisable()" is a puzzle. IMHO, str_yes_no(), str_on_off(), str_enable_disable() are a good compromise. The main motivation should be code readability. You write the code once. But many people will read it many times. Open coding is sometimes better than misleading macro names. That said, I do not want to block this patchset. If others like it... ;-) > e. One alternative to all of this suggested by Christian König > (43456ba7-c372-84cc-4949-dcb817188e21@amd.com) would be to add a > printk format. But besides the comment, he also seemed to like > the common function. This brought the argument from others that the > simple yesno()/enabledisable() already used in the code is easier to > remember and use than e.g. %py[DOY] Thanks for not going this way :-) > Last patch also has some additional conversion of open coded cases. I > preferred starting with drm/ since this is "closer to home". > > I hope this is a good summary of the previous attempts and a way we can > move forward. > > Andrew Morton, Petr Mladek, Andy Shevchenko: if this is accepted, my > proposal is to take first 2 patches either through mm tree or maybe > vsprintf. Last patch can be taken later through drm. I agree with Andy that it should go via drm tree. It would make it easier to handle potential conflicts. Just in case, you decide to go with str_yes_no() or something similar. Mass changes are typically done at the end on the merge window. The best solution is when it can be done by a script. Best Regards, Petr