diff mbox series

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

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

Commit Message

Alistair Francis Aug. 12, 2020, 2:40 p.m. UTC 
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(-)

Comments

Maciej W. Rozycki Aug. 17, 2020, 1:53 p.m. UTC | #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 Aug. 18, 2020, 5:37 p.m. UTC | #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.
Maciej W. Rozycki Aug. 21, 2020, 5:13 p.m. UTC | #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
diff mbox series

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