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

login
register
mail settings
Submitter Baruch Siach
Date Nov. 7, 2013, 7:03 a.m.
Message ID <0834f308731ac7effd22ea37b885f28b317945db.1383807606.git.baruch@tkos.co.il>
Download mbox | patch
Permalink /patch/289209/
State Superseded
Headers show

Comments

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(-)
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

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)