Re: [patch] Fix 64-bit->32-bit vDSO reporting

[Roland McGrath <roland at hack dot frob dot com>]

> +  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