diff mbox series

[v3,02/19] RISC-V: Cleanup some of the sysdep.h code

Message ID	91a2de374c9ae48869743324f4a0edc185f63161.1594568655.git.alistair.francis@wdc.com
State	Committed
Headers	DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org C4DAC3858D37 IronPort-SDR: YmoFbsQymFVgKCtCz6v2g6uvpnN1HSQolCYb5/wHE05bkmVsP6x1lDWssIY3lXTbw2sunuegrx 4sIIMxqxAQXUG/7Nyg3J3L6fPtw8EjjtfjcJNwcHMyo5N0BWigd9hy1Z9V+IsN7jlGHV6pvQux LjrX67Q8vY1u+3RRLICl7vl5DRjyjCFPNA/RNcXv3EQ+Sb6U0o8Aw+pynoXER8103rT4qYrwXu ii8uA6wW3G3IF9iizIoaLzwR5xh8KY8Rfbbrwfh0pfmHH9vPuezA96g66RiJLG5yqdnWYNQUF0 EGU= IronPort-SDR: f9XPGLVA1f+5hdYiZ877UEQ1PS5udKhifD4elqurZofdGevIYasxcOpE8eG5GNECV7s4ra4UMx 5QcpC9P+IIzDPyjq2YlhVGrpi7Lakq8yc= IronPort-SDR: TpYwsUwI2Tl5ALshBlIJLsXEfsuoAmqGYkeiRLoncPd2zGP9Z+0VX0ga1fWFBjw481VqBRrVQT 2ARaToyi1oyg== WDCIronportException: Internal To: libc-alpha@sourceware.org Subject: [PATCH v3 02/19] RISC-V: Cleanup some of the sysdep.h code Date: Sun, 12 Jul 2020 08:47:26 -0700 Message-Id: <91a2de374c9ae48869743324f4a0edc185f63161.1594568655.git.alistair.francis@wdc.com> In-Reply-To: <cover.1594568655.git.alistair.francis@wdc.com> References: <cover.1594568655.git.alistair.francis@wdc.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Precedence: list From: Alistair Francis via Libc-alpha <libc-alpha@sourceware.org> Reply-To: Alistair Francis <alistair.francis@wdc.com> Cc: alistair.francis@wdc.com Errors-To: libc-alpha-bounces@sourceware.org Sender: "Libc-alpha" <libc-alpha-bounces@sourceware.org>
Series	glibc port for 32-bit RISC-V (RV32) \| [v3,00/19] glibc port for 32-bit RISC-V (RV32) [v3,01/19] RISC-V: Use 64-bit time_t and off_t for RV32 and RV64 [v3,02/19] RISC-V: Cleanup some of the sysdep.h code [v3,03/19] RISC-V: Use 64-bit-time syscall numbers with the 32-bit port [v3,04/19] RISC-V: Add support for 32-bit vDSO calls [v3,05/19] RISC-V: Support dynamic loader for the 32-bit [v3,06/19] sysv/linux: riscv: Fix dl-cache.h indentation [v3,07/19] RISC-V: Add path of library directories for the 32-bit [v3,08/19] RISC-V: Add arch-syscall.h for RV32 [v3,09/19] RISC-V: nptl: update default pthread-offsets.h [v3,10/19] RISC-V: Support the 32-bit ABI implementation [v3,11/19] RISC-V: Hard float support for 32-bit [v3,12/19] RISC-V: Add ABI lists [v3,13/19] RISC-V: Add the RV32 libm-test-ulps [v3,14/19] RISC-V: Fix llrint and llround missing exceptions on RV32 [v3,15/19] RISC-V: Build Infastructure for 32-bit [v3,16/19] riscv32: Specify the arch_minimum_kernel as 5.4 [v3,17/19] RISC-V: Add rv32 path to RTLDLIST in ldd [v3,18/19] Documentation for the RISC-V 32-bit port [v3,19/19] Add RISC-V 32-bit target to build-many-glibcs.py

Commit Message

Alistair Francis July 12, 2020, 3:47 p.m. UTC

  ---
 sysdeps/unix/sysv/linux/riscv/sysdep.h | 14 +-------------
 1 file changed, 1 insertion(+), 13 deletions(-)

Comments

Maciej W. Rozycki July 16, 2020, 1:07 a.m. UTC | #1

On Sun, 12 Jul 2020, Alistair Francis via Libc-alpha wrote:

> ---
>  sysdeps/unix/sysv/linux/riscv/sysdep.h | 14 +-------------
>  1 file changed, 1 insertion(+), 13 deletions(-)

 Please try to have at least a little explanation for posterity with 
non-mechanical changes like this, e.g.:

Remove a duplicate inclusion of <sysdeps/unix/sysdep.h> which is already 
pulled via <sysdeps/unix/sysv/linux/generic/sysdep.h>, and the inclusion 
of <errno.h> whose definition of `__set_errno' is not needed here.

It's not always easy for the reader to reproduce an environment needed to 
bring the observations that have lead to the conclusion a given change is 
the right one to make; they shouldn't have to anyway.

> diff --git a/sysdeps/unix/sysv/linux/riscv/sysdep.h b/sysdeps/unix/sysv/linux/riscv/sysdep.h
> index 83e4adf6a2..429686cebe 100644
> --- a/sysdeps/unix/sysv/linux/riscv/sysdep.h
> +++ b/sysdeps/unix/sysv/linux/riscv/sysdep.h
> @@ -107,19 +107,7 @@
>  # undef ret_ERRVAL
>  # define ret_ERRVAL ret
>  
> -#endif /* __ASSEMBLER__ */
> -
> -/* In order to get __set_errno() definition in INLINE_SYSCALL.  */
> -#ifndef __ASSEMBLER__
> -# include <errno.h>
> -#endif
> -
> -#include <sysdeps/unix/sysdep.h>
> -
> -#undef SYS_ify
> -#define SYS_ify(syscall_name)	__NR_##syscall_name

 This redefinition actually has to stay; sorry to be unclear about it.  I 
can see you have readded it with 03/19, but having things broken even for 
the duration of a single commit will interfere with bisection.

 Please place it after the common inclusions at the top of this file.

  Maciej

Alistair Francis Aug. 10, 2020, 3:16 p.m. UTC | #2

On Wed, Jul 15, 2020 at 6:07 PM Maciej W. Rozycki via Libc-alpha
<libc-alpha@sourceware.org> wrote:
>
> On Sun, 12 Jul 2020, Alistair Francis via Libc-alpha wrote:
>
> > ---
> >  sysdeps/unix/sysv/linux/riscv/sysdep.h | 14 +-------------
> >  1 file changed, 1 insertion(+), 13 deletions(-)
>
>  Please try to have at least a little explanation for posterity with
> non-mechanical changes like this, e.g.:
>
> Remove a duplicate inclusion of <sysdeps/unix/sysdep.h> which is already
> pulled via <sysdeps/unix/sysv/linux/generic/sysdep.h>, and the inclusion
> of <errno.h> whose definition of `__set_errno' is not needed here.

Added!

>
> It's not always easy for the reader to reproduce an environment needed to
> bring the observations that have lead to the conclusion a given change is
> the right one to make; they shouldn't have to anyway.
>
> > diff --git a/sysdeps/unix/sysv/linux/riscv/sysdep.h b/sysdeps/unix/sysv/linux/riscv/sysdep.h
> > index 83e4adf6a2..429686cebe 100644
> > --- a/sysdeps/unix/sysv/linux/riscv/sysdep.h
> > +++ b/sysdeps/unix/sysv/linux/riscv/sysdep.h
> > @@ -107,19 +107,7 @@
> >  # undef ret_ERRVAL
> >  # define ret_ERRVAL ret
> >
> > -#endif /* __ASSEMBLER__ */
> > -
> > -/* In order to get __set_errno() definition in INLINE_SYSCALL.  */
> > -#ifndef __ASSEMBLER__
> > -# include <errno.h>
> > -#endif
> > -
> > -#include <sysdeps/unix/sysdep.h>
> > -
> > -#undef SYS_ify
> > -#define SYS_ify(syscall_name)        __NR_##syscall_name
>
>  This redefinition actually has to stay; sorry to be unclear about it.  I
> can see you have readded it with 03/19, but having things broken even for
> the duration of a single commit will interfere with bisection.
>
>  Please place it after the common inclusions at the top of this file.

Fixed.

Alistair

>
>   Maciej

diff mbox series

Patch

diff --git a/sysdeps/unix/sysv/linux/riscv/sysdep.h b/sysdeps/unix/sysv/linux/riscv/sysdep.h
index 83e4adf6a2..429686cebe 100644
--- a/sysdeps/unix/sysv/linux/riscv/sysdep.h
+++ b/sysdeps/unix/sysv/linux/riscv/sysdep.h
@@ -107,19 +107,7 @@ 
 # undef ret_ERRVAL
 # define ret_ERRVAL ret
 
-#endif /* __ASSEMBLER__ */
-
-/* In order to get __set_errno() definition in INLINE_SYSCALL.  */
-#ifndef __ASSEMBLER__
-# include <errno.h>
-#endif
-
-#include <sysdeps/unix/sysdep.h>
-
-#undef SYS_ify
-#define SYS_ify(syscall_name)	__NR_##syscall_name
-
-#ifndef __ASSEMBLER__
+#else
 
 # define VDSO_NAME  "LINUX_4.15"
 # define VDSO_HASH  182943605