linux-kselftest.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Zhangjin Wu <falcon@tinylab.org>
To: w@1wt.eu
Cc: falcon@tinylab.org, arnd@arndb.de, david.laight@aculab.com,
	linux-kernel@vger.kernel.org, linux-kselftest@vger.kernel.org,
	thomas@t-8ch.de, tanyuan@tinylab.org
Subject: [RFC] tools/nolibc: replace duplicated -ENOSYS return with single -ENOSYS return
Date: Sun, 27 Aug 2023 16:32:25 +0800	[thread overview]
Message-ID: <20230827083225.7534-1-falcon@tinylab.org> (raw)

Hi, Willy

Since we have already finished the size inflate regression task [1], to share
and discuss the progress about the -ENOSYS return work, here launchs a new
thread, it is split from [2].

[1]: https://lore.kernel.org/lkml/ZNtszQeigYuItaKA@1wt.eu/
[2]: https://lore.kernel.org/lkml/20230814172233.225944-1-falcon@tinylab.org/#R

This is only for brain storming, it is far from a solution ;-)

> 
> > [...]
> > > > 
> > > >     /* __systry2() is used to select one of two provided low level syscalls */
> > > >     #define __systry2(a, sys_a, sys_b) \
> > > >     	((NOLIBC__NR_##a != NOLIBC__NR_NOSYS) ? (sys_a) : (sys_b))
> > > 
> > > But this supposes that all of them are manually defined as you did above.
> > > I'd rather implement an ugly is_numeric() macro based on argument
> > > resolution. I've done it once in another project, I don't remember
> > > precisely where it is but I vaguely remember that it used to check
> > > that the string resolution of the argument gave a letter (when it
> > > does not exist) or a digit (when it does). I can look into that later
> > > if needed. But please avoid extra macro definitions as much as possible,
> > > they're a real pain to handle in the code. There's no error when one is
> > > missing or has a typo, it's difficult to follow them and they don't
> > > appear in the debugger.
> > >
> > 
> > Yeah, your reply inspired me to look into the IS_ENABLED() from
> > ../include/linux/kconfig.h macro again, there was a __is_defined() there, let's
> > throw away the ugly sysnr.h. I thought of IS_ENABLED() was only for y/n/m
> > before, but it does return 0 when the macro is not defined, it uses the same
> > trick in syscall() to calculate the number of arguments, if the macro is not
> > defined, then, 0 "argument".
> >
> 
> The above trick is only for ""#define something 1" ;-)
>

Here shares a little progress on this, I have found it is easy to implement an
ugly is_numeric() like macro as following:

    /* Imported from include/linux/stringify.h */
    #define __stringify_1(x...)     #x
    #define __stringify(x...)       __stringify_1(x)

    /*
     * Check __NR_* definition by stringizing
     *
     * - The stringizing is to silence compile error about undefined macro
     * - If defined, the result looks like "3", "(4000 + 168)", not begin with '_'
     * - If not defined, the result looks like "__NR_read", begins with '_'
     */

    #define __is_nr_defined(nr)     ___is_nr_defined(__stringify(nr))
    #define ___is_nr_defined(str)   (str[0] != '_')

__is_nr_defined() is able to check if __NR_xxx is defined, but the harder part
is getting the number of defined __NR_* without the error about undefined
macro.

Of course, we can also use the __stringify() trick to do so, but it is
expensive (bigger size, worse performance) to unstringify and get the number
again, the expensive atoi() 'works' for the numeric __NR_*, but not work for
(__NR_*_base + offset) like __NR_* definitions (used by ARM and MIPS), a simple
interpreter is required for such cases and it is more expensive than atoi().

    /* not for ARM and MIPS */

    static int atoi(const char *s);
    #define __get_nr(name)          __nr_atoi(__stringify(__NR_##name))
    #define __nr_atoi(str)          (str[0] == '_' ? -1L : ___nr_atoi(str))
    #define ___nr_atoi(str)         (str[0] == '(' ? -1L : atoi(str))

Welcome more discussion or let's simply throw away this direction ;-)

But it may really help us to drop tons of duplicated code pieces like this:

    #ifdef __NR_xxxx
    ...
    #else
        return -ENOSYS;
    #endif

David, Thomas and Arnd, any inspiration on this, or is this really impossible
(or make things worse) in language level? ;-)

What I'm thinking about is something like this or similar (As Willy commented
before, the __sysdef() itself is not that good, please ignore itself, the core
target here is using a single -ENOSYS return for all of the undefined
branches):

    #define __sysdef(name, ...)     \
    	(__is_nr_defined(__NR_##name) ? my_syscall(__get_nr(name), ##__VA_ARGS__) : (long)-ENOSYS)

Or as Arnd replied in an old email thread before, perhaps the whole #ifdef's
code piece (and even the input types and return types of sys_*) above can be
generated from .tbl or the generic unistd.h automatically in the sysroot
installation stage?

BR,
Zhangjin

             reply	other threads:[~2023-08-27  8:33 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-08-27  8:32 Zhangjin Wu [this message]
2023-08-27  9:17 ` [RFC] tools/nolibc: replace duplicated -ENOSYS return with single -ENOSYS return Thomas Weißschuh
2023-08-29  6:29   ` Willy Tarreau
2023-08-30  0:19     ` Zhangjin Wu
2023-08-30  3:25       ` Willy Tarreau
2023-09-01 19:03         ` Zhangjin Wu
2023-09-03  9:31           ` Willy Tarreau
2023-08-27 21:51 ` David Laight
2023-08-28  9:46   ` David Laight
2023-08-29 23:39     ` Zhangjin Wu
2023-08-30 14:40   ` Ammar Faizi
2023-08-30 16:21     ` David Laight
2023-08-31  7:41       ` Zhangjin Wu
2023-08-31  8:37         ` David Laight
2023-09-01 19:48           ` Zhangjin Wu

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20230827083225.7534-1-falcon@tinylab.org \
    --to=falcon@tinylab.org \
    --cc=arnd@arndb.de \
    --cc=david.laight@aculab.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-kselftest@vger.kernel.org \
    --cc=tanyuan@tinylab.org \
    --cc=thomas@t-8ch.de \
    --cc=w@1wt.eu \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).