This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] Use "_" prefix for macro variables, and harmonize variable names for sysdep syscall macros.


On Thu, 11 Jun 2015, Carlos O'Donell wrote:

> diff --git a/sysdeps/unix/sysv/linux/mips/mips32/sysdep.h b/sysdeps/unix/sysv/linux/mips/mips32/sysdep.h
> index a2aa38d..231c80a 100644
> --- a/sysdeps/unix/sysv/linux/mips/mips32/sysdep.h
> +++ b/sysdeps/unix/sysv/linux/mips/mips32/sysdep.h
> @@ -50,13 +50,13 @@
>  #undef INLINE_SYSCALL
>  #define INLINE_SYSCALL(name, nr, args...)                               \
>    ({ INTERNAL_SYSCALL_DECL (_sc_err);					\
> -     long result_var = INTERNAL_SYSCALL (name, _sc_err, nr, args);	\
> -     if ( INTERNAL_SYSCALL_ERROR_P (result_var, _sc_err) )		\
> +     long _sc_ret = INTERNAL_SYSCALL (name, _sc_err, nr, args);		\
> +     if ( INTERNAL_SYSCALL_ERROR_P (_sc_ret, _sc_err) )			\
>         {								\
> -	 __set_errno (INTERNAL_SYSCALL_ERRNO (result_var, _sc_err));	\
> -	 result_var = -1L;						\
> +	 __set_errno (INTERNAL_SYSCALL_ERRNO (_sc_ret, _sc_err));	\
> +	 _sc_ret = -1L;							\
>         }								\
> -     result_var; })
> +     _sc_ret; })
>  
>  #undef INTERNAL_SYSCALL_DECL
>  #define INTERNAL_SYSCALL_DECL(err) long err __attribute__ ((unused))
> @@ -137,7 +137,7 @@
>  
>  #define internal_syscall0(v0_init, input, number, err, dummy...)	\
>  ({									\
> -	long _sys_result;						\
> +	long _sc_ret;							\

See what I said in 
<https://sourceware.org/ml/libc-alpha/2015-06/msg00050.html> about 
result_var and _sys_result as separate variables.  INLINE_SYSCALL calls 
INTERNAL_SYSCALL which calls internal_syscall##nr.  So you're introducing 
shadowing here by using the same variable name in different macros one of 
which calls the other.  That is not a good idea (even if in fact the inner 
macro never has occasion to refer to the outer variable).  I don't know if 
this applies to any of the other architectures in this patch, but that 
should be checked.

-- 
Joseph S. Myers
joseph@codesourcery.com


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]