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

Message ID c1e18d8cf073268918f93bff10b44519333625c0.1597243100.git.alistair.francis@wdc.com
State Superseded
Headers show
Series
  • glibc port for 32-bit RISC-V (RV32)
Related show

Commit Message

Carlos O'Donell via Libc-alpha Aug. 12, 2020, 2:40 p.m.
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.
---
 sysdeps/unix/sysv/linux/riscv/sysdep.h | 17 ++++-------------
 1 file changed, 4 insertions(+), 13 deletions(-)

-- 
2.27.0

Comments

Carlos O'Donell via Libc-alpha Aug. 17, 2020, 1:53 p.m. | #1
On Wed, 12 Aug 2020, Alistair Francis wrote:

> diff --git a/sysdeps/unix/sysv/linux/riscv/sysdep.h b/sysdeps/unix/sysv/linux/riscv/sysdep.h

> index 83e4adf6a2..fbc2436691 100644

> --- a/sysdeps/unix/sysv/linux/riscv/sysdep.h

> +++ b/sysdeps/unix/sysv/linux/riscv/sysdep.h

[...]
> @@ -107,19 +110,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


 Please comment #else:

#else /* !__ASSEMBLER__ */

especially as the conditional part above is very long.  Otherwise OK.

 This is not actually mentioned in our coding style document, unlike 
#endif, giving an impression we don't want #else statements commented: 
<https://sourceware.org/glibc/wiki/Style_and_Conventions#Commenting_.23endif>.  
This is however covered by the generic GNU Coding Standards document:
<https://www.gnu.org/prep/standards/standards.html#Comments>.

 Carlos: Do we want our wiki updated here?  ISTM we should.  WDYT?

  Maciej
Carlos O'Donell via Libc-alpha Aug. 18, 2020, 5:37 p.m. | #2
On 8/17/20 9:53 AM, Maciej W. Rozycki wrote:
> On Wed, 12 Aug 2020, Alistair Francis wrote:

> 

>> diff --git a/sysdeps/unix/sysv/linux/riscv/sysdep.h b/sysdeps/unix/sysv/linux/riscv/sysdep.h

>> index 83e4adf6a2..fbc2436691 100644

>> --- a/sysdeps/unix/sysv/linux/riscv/sysdep.h

>> +++ b/sysdeps/unix/sysv/linux/riscv/sysdep.h

> [...]

>> @@ -107,19 +110,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

> 

>  Please comment #else:

> 

> #else /* !__ASSEMBLER__ */

> 

> especially as the conditional part above is very long.  Otherwise OK.

> 

>  This is not actually mentioned in our coding style document, unlike 

> #endif, giving an impression we don't want #else statements commented: 

> <https://sourceware.org/glibc/wiki/Style_and_Conventions#Commenting_.23endif>.  

> This is however covered by the generic GNU Coding Standards document:

> <https://www.gnu.org/prep/standards/standards.html#Comments>.

> 

>  Carlos: Do we want our wiki updated here?  ISTM we should.  WDYT?


Yes!

The GNU Coding Standard explains that we should comment #else, but doesn't
say what kind of comment should be used, and that content is up to the
project.

The section should cover how glibc is unique and what we expect from the
#else comment.

Your suggestion looks good to me.

-- 
Cheers,
Carlos.
Carlos O'Donell via Libc-alpha Aug. 21, 2020, 5:13 p.m. | #3
On Tue, 18 Aug 2020, Carlos O'Donell wrote:

> >  Please comment #else:

> > 

> > #else /* !__ASSEMBLER__ */

> > 

> > especially as the conditional part above is very long.  Otherwise OK.

> > 

> >  This is not actually mentioned in our coding style document, unlike 

> > #endif, giving an impression we don't want #else statements commented: 

> > <https://sourceware.org/glibc/wiki/Style_and_Conventions#Commenting_.23endif>.  

> > This is however covered by the generic GNU Coding Standards document:

> > <https://www.gnu.org/prep/standards/standards.html#Comments>.

> > 

> >  Carlos: Do we want our wiki updated here?  ISTM we should.  WDYT?

> 

> Yes!

> 

> The GNU Coding Standard explains that we should comment #else, but doesn't

> say what kind of comment should be used, and that content is up to the

> project.

> 

> The section should cover how glibc is unique and what we expect from the

> #else comment.

> 

> Your suggestion looks good to me.


 I have updated the wiki then, thank you for your opinion.

 Also we don't explicitly mention #elif, but I think it can be implied 
that the convention followed for conditional parts beyond that directive 
is the same as with plain #if.

  Maciej

Patch

diff --git a/sysdeps/unix/sysv/linux/riscv/sysdep.h b/sysdeps/unix/sysv/linux/riscv/sysdep.h
index 83e4adf6a2..fbc2436691 100644
--- a/sysdeps/unix/sysv/linux/riscv/sysdep.h
+++ b/sysdeps/unix/sysv/linux/riscv/sysdep.h
@@ -22,6 +22,9 @@ 
 #include <sysdeps/unix/sysv/linux/generic/sysdep.h>
 #include <tls.h>
 
+#undef SYS_ify
+#define SYS_ify(syscall_name)	__NR_##syscall_name
+
 #ifdef __ASSEMBLER__
 
 # include <sys/asm.h>
@@ -107,19 +110,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