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