linux-riscv.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
From: Zhangjin Wu <falcon@tinylab.org>
To: arnd@arndb.de
Cc: falcon@tinylab.org, linux-kernel@vger.kernel.org,
	linux-kselftest@vger.kernel.org, linux-riscv@lists.infradead.org,
	thomas@t-8ch.de, w@1wt.eu
Subject: Re: [PATCH v3 1/3] tools/nolibc: fix up #error compile failures with -ENOSYS
Date: Wed,  7 Jun 2023 17:46:15 +0800	[thread overview]
Message-ID: <20230607094615.792013-1-falcon@tinylab.org> (raw)
In-Reply-To: <b80f021a-30b7-47e9-8455-3844be157cc2@app.fastmail.com>

> On Wed, Jun 7, 2023, at 07:19, Zhangjin Wu wrote:
> >> On Sat, Jun 3, 2023, at 11:01, Zhangjin Wu wrote:
> >
> > Yeah, agreed, we can clean up them one by one, If split them to their own
> > syscalls, I have two questions (for Arnd, and Willy too):
> >
> > 1. do we need to add the corresponding library routines at the same time?
> >
> >   Use llseek() as an example, there will be llseek() and lsee64(). If off_t
> >   would be converted to 64bit, then, they can be simply added as follows:
> >
> >     #define lseek64 lseek
> >     #define llseek lseek
> >
> >   Or something like this:
> >
> >     static __attribute__((unused))
> >     loff_t lseek(int fd, loff_t offset, int whence)
> >     {
> >     	return lseek(fd, offset, whence);
> >     }
> >
> >     static __attribute__((unused))
> >     off64_t lseek(int fd, off64_t offset, int whence)
> >     {
> >     	return lseek(fd, offset, whence);
> >     }
> >
> >   This one aligns with the other already added library routines.
> >
> >   Which one do you like more?
> 
> lseek() is probably not a good example, as the llseek and lseek64
> library functions are both nonstandard, and I'd suggest leaving them
> out of nolibc altogether.
>

Ok, agree, as the 64bit version of lseek may be enough for nolibc, if a target
application really require, they can add the alias themselves.

> Are there any examples of functions where we actually want mulitple
> versions?
>

For example, the following ones are related to the syscalls being added,
all of them have similar library routines in current sys.h:

* waitid, https://linux.die.net/man/2/waitid
* ppoll, https://linux.die.net/man/2/ppoll
* pselect, https://linux.die.net/man/2/pselect6
* clock_gettime, https://linux.die.net/man/2/clock_gettime

The similar routines are put in right side:

* waitid --> waitpid, wait, wait4
* ppoll --> poll
* pselect --> select
* clock_gettime --> gettimeofday

For the clock_gettime, it may also let us think about if we need to add
its friends (clock_getres, clock_settime) together.

> > 2. If so, how to cope with the new types when add the library routines?
> >
> >   Still use the above llseek() as an example, If not use '#define' method,
> >   We may need to declare loff_t and off64_t in std.h too:
> >
> >     #define off64_t off_t
> >     #define loff_t off_t
> >
> >   Or align with the other new types, use 'typedef' instead of '#define'.
> 
> If we do want to include the explicit off64_t interfaces from glibc,
> I'd suggest doing it the same way as musl:
> https://git.musl-libc.org/cgit/musl/tree/include/unistd.h#n201
>

Thanks.

> >
> >> This patch is a step in that direction though, so I think that's
> >> totally fine.
> >
> > Thanks, so, can I pick your Reviewed-by for the first two patches? I'm ready to
> > send v4 now ;-)
> 
> Yes, please do.
>

Thanks very much, just added the Reviewed-by lines in v4 and have
already sent v4 out, welcome your review on the revision of the 3rd one,
it is almost consistent with the original Makefile now.

Best regards,
Zhangjin

>      Arnd

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

  reply	other threads:[~2023-06-07  9:46 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-06-03  9:00 [PATCH v3 0/3] nolibc: add part2 of support for rv32 Zhangjin Wu
2023-06-03  9:01 ` [PATCH v3 1/3] tools/nolibc: fix up #error compile failures with -ENOSYS Zhangjin Wu
2023-06-06  7:35   ` Arnd Bergmann
2023-06-07  5:19     ` Zhangjin Wu
2023-06-07  8:45       ` Arnd Bergmann
2023-06-07  9:46         ` Zhangjin Wu [this message]
2023-06-07 10:02           ` Arnd Bergmann
2023-06-07 13:26             ` Zhangjin Wu
2023-06-03  9:04 ` [PATCH v3 2/3] tools/nolibc: fix up undeclared syscall macros with #ifdef and -ENOSYS Zhangjin Wu
2023-06-03  9:05 ` [PATCH v3 3/3] selftests/nolibc: riscv: customize makefile for rv32 Zhangjin Wu
2023-06-06  7:43   ` Arnd Bergmann
2023-06-06 11:12     ` Zhangjin Wu
2023-06-06 11:21       ` Arnd Bergmann
2023-06-06 12:07         ` Zhangjin Wu
2023-06-07  1:20           ` Zhangjin Wu
2023-06-07  4:17             ` Willy Tarreau
2023-06-07  6:33               ` Zhangjin Wu
2023-06-07  7:33                 ` Willy Tarreau
2023-06-07  8:11                   ` Zhangjin Wu
2023-06-07 10:44                     ` Willy Tarreau
2023-06-06  4:25 ` [PATCH v3 0/3] nolibc: add part2 of support " Zhangjin Wu
2023-06-06  4:42   ` Willy Tarreau
2023-06-06  6:34     ` Zhangjin Wu
2023-06-06  6:45       ` Willy Tarreau

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=20230607094615.792013-1-falcon@tinylab.org \
    --to=falcon@tinylab.org \
    --cc=arnd@arndb.de \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-kselftest@vger.kernel.org \
    --cc=linux-riscv@lists.infradead.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).