On Sunday 11 January 2009 02:57:56 m m wrote: > On Sun, Jan 11, 2009 at 7:34 AM, Rob Landley <[email protected]> wrote: > > On Saturday 10 January 2009 11:55:55 [email protected] wrote: > >> _dl_read(infile, header, _dl_pagesize); > >> epnt = (ElfW(Ehdr) *) (intptr_t) header; > >> - if (epnt->e_ident[0] != 0x7f || > >> - epnt->e_ident[1] != 'E' || > >> - epnt->e_ident[2] != 'L' || > >> - epnt->e_ident[3] != 'F') > >> + if (epnt->e_ident[EI_MAG0] != ELFMAG0 || > >> + epnt->e_ident[EI_MAG1] != ELFMAG1 || > >> + epnt->e_ident[EI_MAG2] != ELFMAG2 || > >> + epnt->e_ident[EI_MAG3] != ELFMAG3) > > > > Why? The values are never going to change (it would break compatability > > with the entire world), and using the constants you can see what they > > actually are and where they are. > > > > Using macros here only serves to obscure the code to a casual reader. > > I understand those values are never going to change, but IMO you should > use macro instead of raw value as a matter of the coding style.
Why? What does that get you, and where does it end? You've just replaced raw values that mean something (hey, the actual array positions and the actual values that get put into them are the string "\x7fELF", would be more efficient to do a string operation there or not?) with opaque macros that mean nothing. In order to understand the code, you now need to look in two places instead of one. > If you dont like macros in this part, then rather dont look what > went finally to the trunk :) I'm aware that bad changes to the code go in all the time, and that speaking up about them is no guarantee they won't go in. This was a bad change. Rob _______________________________________________ uClibc mailing list [email protected] http://lists.busybox.net/mailman/listinfo/uclibc
