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 show
Series glibc port for 32-bit RISC-V (RV32) | expand

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