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 1/6] Add INLINE_SYSCALL_RETURN/INLINE_SYSCALL_ERROR_RETURN


On Fri, Aug 21, 2015 at 10:17 AM, Joseph Myers <joseph@codesourcery.com> wrote:
> On Fri, 21 Aug 2015, Andreas Schwab wrote:
>
>> Joseph Myers <joseph@codesourcery.com> writes:
>>
>> > I suspect the patch would also better be split up into the most mechanical
>> > changes that don't change generated code at all, and the less obvious
>> > pieces needing more careful review.
>>
>> Especially nothing should be committed until the whole series is aggreed
>> upon.
>
> I don't think that's a good general principle for patch series - a patch
> may well be good on its own merits independent of subsequent patches based
> on it.  But in the case of patch 1/6 here, I agree it only makes sense if
> subsequent patches are agreed.
>
> Specifically, I don't see any need for either of the new macros.  So I
> think the whole basis of this first patch is flawed, since its
> justification is for use in the subsequent patches.
>
> Instead of having two new macros, I think the i386 INLINE_SYSCALL could be
> made to use __syscall_error by making its contents look like
> (appropriately formatted):
>
> ({
> unsigned int resultvar = INTERNAL_SYSCALL (name, , nr, args);
> __glibc_unlikely (INTERNAL_SYSCALL_ERROR_P (resultvar, )) ?
> __syscall_error (resultvar) : resultvar;
> })

The return type of  __syscall_error  will be wrong in this case.
That is the ONLY reason why I added:

/* Similar to INLINE_SYSCALL, but with return type.  It should only be
   used as function return value.  */
#ifndef INLINE_SYSCALL_RETURN
#define INLINE_SYSCALL_RETURN(name, nr, type, args...) \
  INLINE_SYSCALL (name, nr, args)
#endif

so that I can do

({
unsigned int resultvar = INTERNAL_SYSCALL (name, , nr, args);
__glibc_unlikely (INTERNAL_SYSCALL_ERROR_P (resultvar, )) ?
(type) __syscall_error (resultvar) : resultvar;
})

But I don't want to add the new argument to INLINE_SYSCALL
when there is no benefit to call __syscall_error.

-- 
H.J.


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