RE: [PATCH v2 2/2] Add MicroBlaze Port

[David Holsgrove <david dot holsgrove at xilinx dot com>]

Hi Andreas,

> -----Original Message-----
> From: Andreas Schwab [mailto:schwab@suse.de]
> Sent: Tuesday, 9 April 2013 6:32 pm
> To: David Holsgrove
> Cc: libc-ports@sourceware.org; Roland McGrath; Joseph Myers; John Williams;
> Edgar Iglesias; Vinod Kathail; Vidhumouli Hunsigida; Nagaraju Mekala; Tom Shui
> Subject: Re: [PATCH v2 2/2] Add MicroBlaze Port
> 
> David Holsgrove <david.holsgrove@xilinx.com> writes:
> 
> > +inline long
> > +get_frame_size (unsigned long instr)
> > +{
> > +    return abs ((short signed) (instr & 0xFFFF));
> > +}
> > +
> > +static unsigned long *
> > +find_frame_creation (unsigned long *pc)
> > +{
> > +    int i;
> 
> Two spaces indent.
> 
> > +
> > +    /* NOTE: Distance to search is arbitrary
> > +     *   250 works well for most things,
> > +     *   750 picks up things like tcp_recvmsg(),
> > +     *  1000 needed for fat_fill_super().  */
> 
> No initial * in comments.  No () when referring to functions, instead of
> actual calls.
> 
> > +            if ((!(*pprev_pc) || ((*pprev_pc) & 3)))
> 
> The innermost parens are confusing and redundant, likewise for the outer
> pair.
> 
> > +    array[0]=(void *)pc;
> 
> Space around operators and after the cast.
> 
> > +#define atomic_compare_and_exchange_val_acq(mem, newval, oldval)
> \
> > +  ({                                                                          \
> > +    __typeof (*(mem)) __result;                                               \
> > +    if (sizeof (*mem) == 4)                                                   \
> > +      __result = __arch_compare_and_exchange_val_32_acq(mem, newval,
> oldval); \
> 
> Space before open paren.
> 

Thanks for the comments - I've fixed and given another pass over the whole patch,
corrected some backslash alignments and believe it should have cleanly formatted
comments throughout.

Updated patch attached.

thanks again,
David

> Andreas.
> 
> --
> Andreas Schwab, SUSE Labs, schwab@suse.de
> GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
> "And now for something completely different."

Attachment: 0002-MicroBlaze-Port.patch
Description: 0002-MicroBlaze-Port.patch