[v2,04/16] Use msgsnd syscall for Linux implementation

Message ID 1478114813-3526-5-git-send-email-adhemerval.zanella@linaro.org
State Dropped
Headers

Commit Message

Adhemerval Zanella Nov. 2, 2016, 7:26 p.m. UTC
  Changes from previous version:

  - Use __ASSUME_SYSVIPC_SYSCALL instead of __NR_syscall to issue the
    wired syscall or the ipc one.

--

This patch add a direct call to msgsnd syscall if it is supported by
kernel features.

hecked on x86_64, i686, powerpc64le, aarch64, and armhf.

	* sysdeps/unix/sysv/linux/msgsnd.c (__libc_msgsnd): Use msgsnd syscall
	if defined.
---
 ChangeLog                        | 3 +++
 sysdeps/unix/sysv/linux/msgsnd.c | 7 ++++---
 2 files changed, 7 insertions(+), 3 deletions(-)
  

Comments

Yury Norov Nov. 3, 2016, 2:14 p.m. UTC | #1
Hi Adhemerval,

On Wed, Nov 02, 2016 at 05:26:41PM -0200, Adhemerval Zanella wrote:
> Changes from previous version:
> 
>   - Use __ASSUME_SYSVIPC_SYSCALL instead of __NR_syscall to issue the
>     wired syscall or the ipc one.
> 
> --
> 
> This patch add a direct call to msgsnd syscall if it is supported by
> kernel features.
> 
> hecked on x86_64, i686, powerpc64le, aarch64, and armhf.

Typo?

> 
> 	* sysdeps/unix/sysv/linux/msgsnd.c (__libc_msgsnd): Use msgsnd syscall
> 	if defined.
> ---
>  ChangeLog                        | 3 +++
>  sysdeps/unix/sysv/linux/msgsnd.c | 7 ++++---
>  2 files changed, 7 insertions(+), 3 deletions(-)
> 
> diff --git a/sysdeps/unix/sysv/linux/msgsnd.c b/sysdeps/unix/sysv/linux/msgsnd.c
> index 1457e43..4ed3a64 100644
> --- a/sysdeps/unix/sysv/linux/msgsnd.c
> +++ b/sysdeps/unix/sysv/linux/msgsnd.c
> @@ -16,17 +16,18 @@
>     License along with the GNU C Library; if not, see
>     <http://www.gnu.org/licenses/>.  */
>  
> -#include <errno.h>
>  #include <sys/msg.h>
>  #include <ipc_priv.h>
> -
>  #include <sysdep-cancel.h>
> -#include <sys/syscall.h>

IIUC, ipc_priv.h is only needed if__ASSUME_SYSVIPC_SYSCALL is not
defined.

Is sys/msg.h needed for __ASSUME_SYSVIPC_SYSCALL only? If so, it's
better to #include kernel-features.h instead.

The same for other patches of series.

Yury.

>  
>  int
>  __libc_msgsnd (int msqid, const void *msgp, size_t msgsz, int msgflg)
>  {
> +#ifdef __ASSUME_SYSVIPC_SYSCALL
> +  return SYSCALL_CANCEL (msgsnd, msqid, msgp, msgsz, msgflg);
> +#else
>    return SYSCALL_CANCEL (ipc, IPCOP_msgsnd, msqid, msgsz, msgflg,
>  			 (void *) msgp);
> +#endif
>  }
>  weak_alias (__libc_msgsnd, msgsnd)
> -- 
> 2.7.4
  
Adhemerval Zanella Nov. 3, 2016, 8:26 p.m. UTC | #2
On 03/11/2016 12:14, Yury Norov wrote:
> Hi Adhemerval,
> 
> On Wed, Nov 02, 2016 at 05:26:41PM -0200, Adhemerval Zanella wrote:
>> Changes from previous version:
>>
>>   - Use __ASSUME_SYSVIPC_SYSCALL instead of __NR_syscall to issue the
>>     wired syscall or the ipc one.
>>
>> --
>>
>> This patch add a direct call to msgsnd syscall if it is supported by
>> kernel features.
>>
>> hecked on x86_64, i686, powerpc64le, aarch64, and armhf.
> 
> Typo?

Yeap, it is suppose to be 'Checked'.

> 
>>
>> 	* sysdeps/unix/sysv/linux/msgsnd.c (__libc_msgsnd): Use msgsnd syscall
>> 	if defined.
>> ---
>>  ChangeLog                        | 3 +++
>>  sysdeps/unix/sysv/linux/msgsnd.c | 7 ++++---
>>  2 files changed, 7 insertions(+), 3 deletions(-)
>>
>> diff --git a/sysdeps/unix/sysv/linux/msgsnd.c b/sysdeps/unix/sysv/linux/msgsnd.c
>> index 1457e43..4ed3a64 100644
>> --- a/sysdeps/unix/sysv/linux/msgsnd.c
>> +++ b/sysdeps/unix/sysv/linux/msgsnd.c
>> @@ -16,17 +16,18 @@
>>     License along with the GNU C Library; if not, see
>>     <http://www.gnu.org/licenses/>.  */
>>  
>> -#include <errno.h>
>>  #include <sys/msg.h>
>>  #include <ipc_priv.h>
>> -
>>  #include <sysdep-cancel.h>
>> -#include <sys/syscall.h>
> 
> IIUC, ipc_priv.h is only needed if__ASSUME_SYSVIPC_SYSCALL is not
> defined.
> 
> Is sys/msg.h needed for __ASSUME_SYSVIPC_SYSCALL only? If so, it's
> better to #include kernel-features.h instead.
> 
> The same for other patches of series.

I see it would be simpler and usual to just include 'ipc_priv.h' 
regardless instead of adding inside the if '__ASSUME_SYSVIPC_SYSCALL'
brackets.

And although 'sys/msg.h' is not really required for this specific
file (since we are defining __libc_msgsnd and adding an alias), I
also think it is better to add the header for consistency.
  

Patch

diff --git a/sysdeps/unix/sysv/linux/msgsnd.c b/sysdeps/unix/sysv/linux/msgsnd.c
index 1457e43..4ed3a64 100644
--- a/sysdeps/unix/sysv/linux/msgsnd.c
+++ b/sysdeps/unix/sysv/linux/msgsnd.c
@@ -16,17 +16,18 @@ 
    License along with the GNU C Library; if not, see
    <http://www.gnu.org/licenses/>.  */
 
-#include <errno.h>
 #include <sys/msg.h>
 #include <ipc_priv.h>
-
 #include <sysdep-cancel.h>
-#include <sys/syscall.h>
 
 int
 __libc_msgsnd (int msqid, const void *msgp, size_t msgsz, int msgflg)
 {
+#ifdef __ASSUME_SYSVIPC_SYSCALL
+  return SYSCALL_CANCEL (msgsnd, msqid, msgp, msgsz, msgflg);
+#else
   return SYSCALL_CANCEL (ipc, IPCOP_msgsnd, msqid, msgsz, msgflg,
 			 (void *) msgp);
+#endif
 }
 weak_alias (__libc_msgsnd, msgsnd)