This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] AmigaOS 4 port contribution


"Joseph S. Myers" <jsm@polyomino.org.uk> writes:

> Start by giving a proper specification of the language extension - I mean
> in terms of textual edits to the C99 standard, not the vague description
> given under the attribute documentation.  You don't, for example, say how
> the Interface object is determined from the pointer to function involved
> in the function call; there appear to be some restrictions on the syntax
> used to create that Interface object, but various things are unclear, e.g.  
> can an Interface value that isn't an lvalue be used (e.g. (foo ?  
> interface1 : interface2).bar()), and in that case is the pointer a pointer
> to a copy?

Ok.  Should this specification be a separate document, or be
integrated in the documentation somehow?


> * I take it the changes to libtool scripts are already in libtool CVS (as 
> required in codingconventions.html)?

Not yet.  Hopefully they can be committed there quickly, but if not my
suggestion would be to skip those changes for now and pull them from
libtool CVS at a later date.


> I doubt your copyright dates, in particular that there have been no
> significant changes since 2001.  Please check them all carefully.

Good point.


> We've been getting rid of references to "GNU CC", don't reintroduce them.
> 
> This source file (amigaos.c) is also missing comments before all the 
> functions.

Will fix.


> > +/* AmigaOS4 specific macros for use with __atribute__((linearvarargs)) */
> > +#if defined(__amigaos__) && defined (__PPC__)
> > +
> > +#define va_startlinear(AP, x)                  \
> 
> You can't unconditionally add macros to <stdarg.h> or other standard
> headers like this, va_* is *not* a reserved namespace.  Please put all
> this in an AmigaOS-specific header rather than touching stdarg.h at all.

The problem is that the implementation of the macros rely on GCC
internals.  Would it be satisfactory if they were declared as __va_*
in this file, and then simply renamed in the AmigaOS-specific headers?

In either case, they are not unconditional, as there is #if
defined(__amigaos__) around them.


> You don't actually document what these macros are meant to do, but why 
> can't va_start act appropriately according to the attributes on the 
> calling function - why do you need va_startlinear at all?

It's mostly for compatibility with existing code.  But I suppose a
#define va_startlinear va_start
in the AmigaOS header could take care of that just as well...


  // Marcus



Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]