X-Authentication-Warning: delorie.com: mail set sender to djgpp-workers-bounces using -f
X-Recipient: djgpp-workers AT delorie DOT com
Date: Sat, 30 Jul 2011 15:45:33 +0300
From: Eli Zaretskii <eliz AT gnu DOT org>
Subject: Re: [PATCH] allow 64 bit host tools when cross compiling
In-reply-to: <CAA2C=vDnictpZNcsF6UV1Did=WPvmvHM9eSQtT8R5fyTr738cA@mail.gmail.com>
To: djgpp-workers AT delorie DOT com
Message-id: <83k4b0c4ya.fsf@gnu.org>
MIME-version: 1.0
Content-type: text/plain; charset=iso-8859-1
X-012-Sender: halo1 AT inter DOT net DOT il
References: <CAA2C=vCTyDxWBgYzoBB31=hoOBHJhL+famn_6XL2eYb3a_3egA AT mail DOT gmail DOT com> <83zkjwcknb DOT fsf AT gnu DOT org> <CAA2C=vBJEgnGapVsNwy7xW9HUCy0Rpxis2P5=05mxBze_qn7vQ AT mail DOT gmail DOT com> <83wrf0chf4 DOT fsf AT gnu DOT org> <CAA2C=vBtiNnx1XPVHw_6Er1uf0_biO3vpaBEa0zeYQuZKh5BxA AT mail DOT gmail DOT com> <83mxfwc634 DOT fsf AT gnu DOT org> <CAA2C=vDnictpZNcsF6UV1Did=WPvmvHM9eSQtT8R5fyTr738cA AT mail DOT gmail DOT com>
Content-Transfer-Encoding: 8bit
X-MIME-Autoconverted: from QUOTED-PRINTABLE to 8bit by delorie.com id p6UCjQIQ022388
Reply-To: djgpp-workers AT delorie DOT com
Errors-To: nobody AT delorie DOT com
X-Mailing-List: djgpp-workers AT delorie DOT com
X-Unsubscribes-To: listserv AT delorie DOT com
Precedence: bulk

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