[v2] librt: fix mq_timed{send,receive} return instructions

Submitted by Baruch Siach on Nov. 7, 2013, 7:03 a.m.

Details

Message ID 0834f308731ac7effd22ea37b885f28b317945db.1383807606.git.baruch@tkos.co.il
State Superseded
Headers show

Commit Message

Baruch Siach Nov. 7, 2013, 7:03 a.m.
Not all architectures use 'ret' as function return instruction. For example,
xtensa usually uses 'retw'. Use the ret_ERRVAL arch dependant macro instead.

Signed-off-by: Baruch Siach <baruch@tkos.co.il>
---
v2: Add ret_ERRVAL to architectures missing it as noted by Bernhard
	Reutner-Fischer.
---
 libc/sysdeps/linux/bfin/sysdep.h  | 2 ++
 libc/sysdeps/linux/cris/sysdep.h  | 2 ++
 libc/sysdeps/linux/metag/sysdep.h | 2 ++
 libc/sysdeps/linux/sparc/sysdep.h | 2 ++
 librt/mq_timedreceive.S           | 2 +-
 librt/mq_timedsend.S              | 2 +-
 6 files changed, 10 insertions(+), 2 deletions(-)

Comments

aldot Nov. 7, 2013, 9:14 a.m.
On 7 November 2013 08:03, Baruch Siach <baruch@tkos.co.il> wrote:
> Not all architectures use 'ret' as function return instruction. For example,
> xtensa usually uses 'retw'. Use the ret_ERRVAL arch dependant macro instead.
>
> Signed-off-by: Baruch Siach <baruch@tkos.co.il>
> ---
> v2: Add ret_ERRVAL to architectures missing it as noted by Bernhard
>         Reutner-Fischer.
> ---
>  libc/sysdeps/linux/bfin/sysdep.h  | 2 ++
>  libc/sysdeps/linux/cris/sysdep.h  | 2 ++
>  libc/sysdeps/linux/metag/sysdep.h | 2 ++
>  libc/sysdeps/linux/sparc/sysdep.h | 2 ++
>  librt/mq_timedreceive.S           | 2 +-
>  librt/mq_timedsend.S              | 2 +-
>  6 files changed, 10 insertions(+), 2 deletions(-)

I was more thinking about putting the default of ret into
common/sysdep.h, make sure to include that in all arch specific
sysdep.h and just re-define it on arches that diverge from the
default?
Baruch Siach Nov. 7, 2013, 9:18 a.m.
Hi Bernhard,

On Thu, Nov 07, 2013 at 10:14:59AM +0100, Bernhard Reutner-Fischer wrote:
> On 7 November 2013 08:03, Baruch Siach <baruch@tkos.co.il> wrote:
> > Not all architectures use 'ret' as function return instruction. For example,
> > xtensa usually uses 'retw'. Use the ret_ERRVAL arch dependant macro instead.
> >
> > Signed-off-by: Baruch Siach <baruch@tkos.co.il>
> > ---
> > v2: Add ret_ERRVAL to architectures missing it as noted by Bernhard
> >         Reutner-Fischer.
> > ---
> >  libc/sysdeps/linux/bfin/sysdep.h  | 2 ++
> >  libc/sysdeps/linux/cris/sysdep.h  | 2 ++
> >  libc/sysdeps/linux/metag/sysdep.h | 2 ++
> >  libc/sysdeps/linux/sparc/sysdep.h | 2 ++
> >  librt/mq_timedreceive.S           | 2 +-
> >  librt/mq_timedsend.S              | 2 +-
> >  6 files changed, 10 insertions(+), 2 deletions(-)
> 
> I was more thinking about putting the default of ret into
> common/sysdep.h, make sure to include that in all arch specific
> sysdep.h and just re-define it on arches that diverge from the
> default?

Sounds much better. I just had the feeling from looking around at the arch 
specific code that duplicating identical definitions is the way to go. I'll 
update and resubmit.

baruch
Rich Felker Nov. 9, 2013, 5:28 a.m.
On Thu, Nov 07, 2013 at 09:03:30AM +0200, Baruch Siach wrote:
> Not all architectures use 'ret' as function return instruction. For example,
> xtensa usually uses 'retw'. Use the ret_ERRVAL arch dependant macro instead.
> 
> Signed-off-by: Baruch Siach <baruch@tkos.co.il>
> ---
> v2: Add ret_ERRVAL to architectures missing it as noted by Bernhard
> 	Reutner-Fischer.
> ---
>  libc/sysdeps/linux/bfin/sysdep.h  | 2 ++
>  libc/sysdeps/linux/cris/sysdep.h  | 2 ++
>  libc/sysdeps/linux/metag/sysdep.h | 2 ++
>  libc/sysdeps/linux/sparc/sysdep.h | 2 ++
>  librt/mq_timedreceive.S           | 2 +-
>  librt/mq_timedsend.S              | 2 +-

What is the sense in having these files written in assembly in the
first place? A proper C file with an inline syscall should compile to
exactly the same thing and be much more maintainable.

Rich

Patch hide | download patch | download mbox

diff --git a/libc/sysdeps/linux/bfin/sysdep.h b/libc/sysdeps/linux/bfin/sysdep.h
index 352fbf4..0de2eba 100644
--- a/libc/sysdeps/linux/bfin/sysdep.h
+++ b/libc/sysdeps/linux/bfin/sysdep.h
@@ -16,6 +16,8 @@ 
 #define ENTRY(sym) .global sym; .type sym, STT_FUNC; sym:
 #define ENDPROC(sym) .size sym, . - sym
 
+#define ret_ERRVAL ret
+
 #endif
 
 #endif
diff --git a/libc/sysdeps/linux/cris/sysdep.h b/libc/sysdeps/linux/cris/sysdep.h
index a034650..94b0fe8 100644
--- a/libc/sysdeps/linux/cris/sysdep.h
+++ b/libc/sysdeps/linux/cris/sysdep.h
@@ -49,6 +49,8 @@ 
 #undef SYS_ify
 #define SYS_ify(syscall_name)   __NR_##syscall_name
 
+#define ret_ERRVAL ret
+
 /* Syntactic details of assembly-code.  */
 
 /* It is *not* generally true that "ELF uses byte-counts for .align, most
diff --git a/libc/sysdeps/linux/metag/sysdep.h b/libc/sysdeps/linux/metag/sysdep.h
index a12f393..274d79c 100644
--- a/libc/sysdeps/linux/metag/sysdep.h
+++ b/libc/sysdeps/linux/metag/sysdep.h
@@ -22,6 +22,8 @@ 
   SYSCALL_ERROR_HANDLER							\
   END (name)
 
+#define ret_ERRVAL ret
+
 #if defined NOT_IN_libc
 # define SYSCALL_ERROR __local_syscall_error
 # ifdef RTLD_PRIVATE_ERRNO
diff --git a/libc/sysdeps/linux/sparc/sysdep.h b/libc/sysdeps/linux/sparc/sysdep.h
index cf3e3af..7a6aa85 100644
--- a/libc/sysdeps/linux/sparc/sysdep.h
+++ b/libc/sysdeps/linux/sparc/sysdep.h
@@ -10,6 +10,8 @@ 
 
 #define LOADSYSCALL(x) mov __NR_##x, %g1
 
+#define ret_ERRVAL ret
+
 #define ENTRY(name)                 \
     .align 4;                       \
     .global C_SYMBOL_NAME(name);    \
diff --git a/librt/mq_timedreceive.S b/librt/mq_timedreceive.S
index 43a5fda..00fecac 100644
--- a/librt/mq_timedreceive.S
+++ b/librt/mq_timedreceive.S
@@ -3,6 +3,6 @@ 
 #error Missing definition of NR_timedreceive needed for cancellation.
 #endif
 PSEUDO(mq_timedreceive, mq_timedreceive, 5)
-ret
+ret_ERRVAL
 PSEUDO_END(mq_timedreceive)
 librt_hidden_def(mq_timedreceive)
diff --git a/librt/mq_timedsend.S b/librt/mq_timedsend.S
index 13d91da..ee8d483 100644
--- a/librt/mq_timedsend.S
+++ b/librt/mq_timedsend.S
@@ -3,6 +3,6 @@ 
 #error Missing definition of NR_timedsend needed for cancellation.
 #endif
 PSEUDO(mq_timedsend, mq_timedsend, 5)
-ret
+ret_ERRVAL
 PSEUDO_END(mq_timedsend)
 librt_hidden_def(mq_timedsend)