This is the mail archive of the
elfutils-devel@sourceware.org
mailing list for the elfutils project.
Re: [patch] Fix 64-bit->32-bit vDSO reporting
- From: Roland McGrath <roland at hack dot frob dot com>
- To: elfutils-devel at lists dot fedorahosted dot org
- Date: Tue, 29 Jan 2013 14:44:33 -0800
- Subject: Re: [patch] Fix 64-bit->32-bit vDSO reporting
> + ssize_t nread = pread_retry (fd, &buf, sizeof buf, 0);
> + close (fd);
> + if (nread != sizeof (buf) || buf[EI_MAG0] != ELFMAG0
Drop parens on sizeof operand, as above.
> +/* Search /proc/PID/auxv for the AT_SYSINFO_EHDR tag.
> +
> + It would be most easier to call get_pid_class and parse everything according
s/most easier/easiest/
> + to the 32-bit or 64-bit class. But this would bring an overhead of syscalls
s/an overhead/the overhead/
> + opening the "/proc/%d/exe" file.
s/opening/to open and read/
> + Therefore this function tries to parse the "/proc/%d/auxv" content both
> + ways, as if it was the 32-bit format and also if it was the 64-bit format.
s/if it was/if it were/g
> + GElf_Addr sysinfo_ehdr64 = 0, sysinfo_ehdr32 = 0;
elfutils style puts only one variable decl on each line.
> + for (unsigned a32i = 0; a32i < nread / sizeof d.a32[0]; a32i++)
Use size_t (and below). When 'unsigned int' is the best type,
write it like that and never just 'unsigned'.
> + bool valid64 = sysinfo_ehdr64 || segment_align64 != dwfl->segment_align;
> + bool valid32 = sysinfo_ehdr32 || segment_align32 != dwfl->segment_align;
Don't use implicit Boolean coercion: write "sysinfo_ehdr32 != 0".
> + unsigned char pid_class = ELFCLASSNONE;
> + if (valid64 && valid32)
> + pid_class = get_pid_class (pid);
> +
> + if (pid_class == ELFCLASS64 || (valid64 && ! valid32))
> + {
> + *sysinfo_ehdr = sysinfo_ehdr64;
> + dwfl->segment_align = segment_align64;
> + }
> + if (pid_class == ELFCLASS32 || (! valid64 && valid32))
> + {
> + *sysinfo_ehdr = sysinfo_ehdr32;
> + dwfl->segment_align = segment_align32;
> + }
> + return 0;
> }
Just for paranoia, it should return an error if neither "if" succeeds.
Thanks,
Roland