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: Add x32 dl-machine.h


> +#include <sysdeps/x86_64/dl-machine.h>
> +
> +#ifndef x32_dl_machine_h
> +#define x32_dl_machine_h

Though this is done inconsistently already, I think for new files we should
be consistent and use an all-caps identifier starting with underscore.

I see no reason the #include has to be outside the guard.  I know it
doesn't hurt since it's separately guarded, but it is unusual to see any
non-comment contents outside a file's guard, so please move it inside.

> +#define RTLD_START asm ("\n\
> +.text\n\
> +	.align 16\n\

Always use .p2align or .balign in new code.

> +	# Pop the original argument count.\n\
> +	movl (%rsp), %edx\n\
> +	addl $4,%esp\n\
> +	# Adjust the stack pointer to skip _dl_skip_args words.\n\
> +	lea (%rsp,%rax,4), %esp\n\

Just use "lea 4(%rsp,%rax,4), %esp" rather than two instructions.

> +	# Push argc back on the stack.\n\
> +	subl $4,%esp\n\

Space after comma.

> +	# And align stack for the _dl_init_internal call. \n\

Excess space before \n.

> +	# Call the function to run the initializers.\n\
> +	call _dl_init_internal@PLT\n\

Why does this need @PLT when the _dl_start call above didn't?
It seems especially odd since it's calling the INTUSE name whose
whole purpose is not to need the PLT.


Thanks,
Roland


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