Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Sun, 2 Jul 2023 20:50:59 +0200
From:   Willy Tarreau <w@1wt.eu>
To:     Zhangjin Wu <falcon@tinylab.org>
Cc:     thomas@t-8ch.de, arnd@arndb.de, david.laight@aculab.com,
        linux-kernel@vger.kernel.org, linux-kselftest@vger.kernel.org,
        linux-riscv@lists.infradead.org
Subject: Re: [PATCH v5 07/14] tools/nolibc: arch-loongarch.h: shrink with
 SYSCALL_CLOBBERLIST
Message-ID: <20230702185059.GG16233@1wt.eu>
References: <cover.1687957589.git.falcon@tinylab.org>
 <a906a8e5bb39d88fee89be8f2bca8bf62307d468.1687957589.git.falcon@tinylab.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <a906a8e5bb39d88fee89be8f2bca8bf62307d468.1687957589.git.falcon@tinylab.org>
User-Agent: Mutt/1.10.1 (2018-07-13)
Precedence: bulk

On Wed, Jun 28, 2023 at 09:22:21PM +0800, Zhangjin Wu wrote:
> my_syscall<N> share a same long clobber list, define a macro for them.
> 
> Signed-off-by: Zhangjin Wu <falcon@tinylab.org>
> ---
>  tools/include/nolibc/arch-loongarch.h | 25 +++++++++++--------------
>  1 file changed, 11 insertions(+), 14 deletions(-)
> 
> diff --git a/tools/include/nolibc/arch-loongarch.h b/tools/include/nolibc/arch-loongarch.h
> index 292d6a58dc87..fbb4844f7993 100644
> --- a/tools/include/nolibc/arch-loongarch.h
> +++ b/tools/include/nolibc/arch-loongarch.h
> @@ -23,6 +23,10 @@
>   */
>  #define __ARCH_WANT_SYS_PSELECT6
>  
> +#define SYSCALL_CLOBBERLIST			\
> +	"memory", "$t0", "$t1", "$t2", "$t3",	\
> +	"$t4", "$t5", "$t6", "$t7", "$t8"
> +

That's a good idea, but please be careful when adding macro definitions,
we're in code that is used by user space we have no control on, and we're
polluting the end user's macro namespace with plenty of names. While one
could argue that it's unlikely that some program already defines and uses
SYSCALL_CLOBBERLIST, actually with low-level code it's fairly possible.

Till now most of the definitions were for stuff that user-space really
needs (e.g. STDIN_FILENO, various integer limits). If we start to declare
random macros for internal use, at least we should probably prefix them
with _NOLIBC_ or something like this to avoid the risk of collision.

Willy