This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH] Use "_" prefix for macro variables, and harmonize variable names for sysdep syscall macros.
- From: "Carlos O'Donell" <carlos at redhat dot com>
- To: Joseph Myers <joseph at codesourcery dot com>
- Cc: GNU C Library <libc-alpha at sourceware dot org>
- Date: Fri, 12 Jun 2015 13:51:03 -0400
- Subject: Re: [PATCH] Use "_" prefix for macro variables, and harmonize variable names for sysdep syscall macros.
- Authentication-results: sourceware.org; auth=none
- References: <557A4A8A dot 5050801 at redhat dot com> <alpine dot DEB dot 2 dot 10 dot 1506121701480 dot 1777 at digraph dot polyomino dot org dot uk>
On 06/12/2015 01:07 PM, Joseph Myers wrote:
> 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.
I had not considered nested calls like this, but you are absolutely
correct. I will review the sysdeps and post a v2 patch.
Cheers,
Carlos.