This is the mail archive of the 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: Add Linux/x32 sysdep.h

> Here is what I implemented.  Since I need to refine SYSCALL_ERROR_HANDLER
> anyway, I use it to define my own error handler.  Does it look OK?
> I noticed that SYSCALL_ERROR_HANDLER has
>   xorl %edx, %edx;				\
>   subq %rax, %rdx;				\
>   movl %edx, (%rcx);				\
>   orq $-1, %rax;				\
>   jmp L(pseudo_end);
> Why not simply "neg %rax"?  

No idea.  It appears it was done that way in the first version of this
file, added by AJ.

> Also why not just "ret" instead of jmp?

Likewise.  This one seems downright wrong, since theoretically the code at
the label might not be just "ret".  That code (i.e. whatever comes after
the PSEUDO macro invocation) was always meant to be only the code for the
success case.

Those two changes are unrelated to x32, so you can submit them separately
and see if anybody has any comment.

> 	* sysdeps/unix/sysv/linux/x86_64/sysdep.h (SYSCALL_SET_ERRNO):
> 	New macro.  Use R*LP on int and pointer.
> 	* sysdeps/unix/sysv/linux/x86_64/x32/llseek.S: New file.
> 	* sysdeps/unix/sysv/linux/x86_64/x32/sysdep.h: Likewise.

This looks fine to me with one nit below.

> +# if defined PIC && defined RTLD_PRIVATE_ERRNO
> +#  define SYSCALL_SET_ERRNO			\
> +  lea rtld_errno(%rip), %RCX_LP;		\
>    xorl %edx, %edx;				\
> -  subq %rax, %rdx;				\
> -  movl %edx, (%rcx);				\
> -  orq $-1, %rax;				\
> -  jmp L(pseudo_end);
> +  sub %RAX_LP, %RDX_LP;				\
> +  movl %edx, (%rcx);

There should be no trailing ; at the end of a macro definition.
(SYSCALL_ERROR_HANDLER is perhaps a special case because it sometimes
expands to nothing at all.)

> +#  define SYSCALL_ERROR_HANDLER			\
> +0:						\

Instead, put a ; here.


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