This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH v4] MIPS support for GNU hash
- From: Joseph Myers <joseph at codesourcery dot com>
- To: Mihailo Stojanovic <mihailo dot stojanovic at rt-rk dot com>
- Cc: <libc-alpha at sourceware dot org>, Adhemerval Zanella <adhemerval dot zanella at linaro dot org>, "Maciej W . Rozycki" <macro at linux-mips dot org>, Carlos O'Donell <carlos at redhat dot com>, Dragan Mladjenovic <dragan dot mladjenovic at rt-rk dot com>, Petar Jovanovic <petar dot jovanovic at rt-rk dot com>
- Date: Wed, 28 Aug 2019 17:27:41 +0000
- Subject: Re: [PATCH v4] MIPS support for GNU hash
- Ironport-sdr: tRYC+c9ObXVG7NSdQfwvQLls3pkdRyxqRquTb0zdlt2969k3mw3jq/5Iccb6Fw2MT4HubbmZzn o2e6PBZldR+buqDi2tnLlxfjLQiMKcRCNZrjneoxjb1YRC/ZZtOsTyuKIF6F12wG78Cb+kf/FE jcgHjG/fUwpyoscjUj0J7IbSewqEYC1jgVbQ+grUfmon45E/bsicW5FUZrtUJbEs86S3/+ZRO7 vphmiX97nrCQ5sW6BZqVptEik6n3JgI9AruUi2frSE3JAQGkSItzdi0HlY+F6AqWbfkvpiDv+O bi8=
- Ironport-sdr: Wx/uCusPjDyoe6zCLF4lnHLiE4vDKb4k+cnzIxosMMJyNfRGCKhnx9Q4WmcP1ovPsboq4lK8rX Q3hDkS9+9yJruLE35neIfPT1YRVPtGr9FwHiG93PgE/UAZzOEptZsNexAkb2X3qn735tb9jqNT WPPsmckFdUYwhbsv/Ax+vVkuZ78w/DorLalziNE8J2xDVtvsl8MAnzF9OGCX3LLzqJNH9+o5oz 162idm9l7HXnVoFjvvlKE0O2XKlrQKrNTFngvd0g78hYfNIMRzQcsAUA5bhvDgZpqBo7cmPN49 X10=
- References: <1567004992-23956-1-git-send-email-mihailo.stojanovic@rt-rk.com>
On Wed, 28 Aug 2019, Mihailo Stojanovic wrote:
> + symndx = ELF_MACHINE_HASH_SYMIDX(match, hasharr);
Missing space before '('.
> +
> + /* Initialize MIPS xhash translation table. */
> + ELF_MACHINE_XHASH_SETUP (hash32, symbias, map)
The normal style is that such macros are written to act syntactically like
a single statement. That means using a semicolon after the macro call.
It means that the macro definition should use the do {} while (0) style
(with appropriate GNU indentation) so that it does behave as a single
statement in any context.
> +/* Setup MIPS_XHASH. */
> +#define ELF_MACHINE_XHASH_SETUP(hash32, symbias, map) \
> + (hash32) += (map)->l_info[DT_MIPS (SYMTABNO)]->d_un.d_val - (symbias); \
> + (map)->l_mach.mips_xlat_zero = (hash32) - (symbias);
So you should have
do
{
/* Statements. */
}
while (0)
as the macro definition.
(I think it's best for the default definition at least to use its
arguments, cast to (void), rather than being completely empty, to improve
the checking done in the non-MIPS case. You should also test the patch on
one non-MIPS platform to verify that case is unaffected.)
--
Joseph S. Myers
joseph@codesourcery.com