Mail Archives: djgpp-workers/2011/07/30/08:45:30
> Date: Sat, 30 Jul 2011 15:36:43 +0300
> From: Ozkan Sezer <sezeroz AT gmail DOT com>
>
> >> +#ifndef _DJ_DEFINED_NATIVE_TYPES
> >> +#define _DJ_DEFINED_NATIVE_TYPES
> >
> > Why are these additional macros needed?
> >
>
> The types are defined in two different headers, so the guard
> prevents multiple definition
Ah, okay. Sorry I missed that.
> >> struct external_lineno {
> >> union {
> >> - unsigned long l_symndx __attribute__((packed)); /* function name symbol index, iff l_lnno == 0 */
> >> - unsigned long l_paddr __attribute__((packed)); /* (physical) address of line number */
> >> + ULONG32 l_symndx; /* function name symbol index, iff l_lnno == 0 */
> >> + ULONG32 l_paddr; /* (physical) address of line number */
> >> } l_addr;
> >> unsigned short l_lnno; /* line number */
> >> -};
> >> +} __attribute__((packed));
> >
> > Why did you remove the `__attribute__((packed))' parts (here and
> > elsewhere)? These structures must be binary-compatible with data
> > produced by utilities outside of DJGPP control, so letting the
> > compiler pad the struct members to its liking might cause subtle bugs.
> >
>
> How is 'long' packed?? The intention there must be
> to pack the structure, not the type itself.
That's true, but I think some versions of the compiler needed the
attribute on the members as well. I'd rather leave them alone, unless
they actually cause trouble.
> >> -#define VALID_RELOC(r) ((r.r_type != RELOC_REL32) && (r.r_symndx != -1UL))
> >> +#define VALID_RELOC(r) ((r.r_type != RELOC_REL32) && (r.r_symndx != 0xFFFFFFFF))
> >
> > I think this is cleaner:
> >
> > #define VALID_RELOC(r) ((r.r_type != RELOC_REL32) && (r.r_symndx != (ULONG32)-1))
> >
> >> - relocs[j].r_symndx = -1UL;
> >> + relocs[j].r_symndx = 0xFFFFFFFF;
> >
> > Same here.
>
> Well, is (LONG32)-1 not the same as 0xffffffff ?
You meant ULONG32, I presume. This is implementation-defined, I think
(whether sign is extended or not), and using -1 shows the intent more
clearly, IMO.
- Raw text -