[Fwd: [PATCH] cr16-elf: position independent code (PIC) support]

M R Swami Reddy MR.Swami.Reddy@nsc.com
Tue Nov 25 11:41:00 GMT 2008


Hi Nick,

Thank you very much for your quick review and comments.

>  > +  /* Random linker state flags.  */
>  > +#define CR16_HASH_ENTRIES_INITIALIZED 0x1
>  > +  char flags;
> 
> Similar questions.

No specific reason. These macros are defined with unique values. I will set with 
0x1 for both macros appropriately.

> The "PARAMS" and "PTR" macros are redundant.  Please always use function
> prototypes and the "void *" type instead.  ie:
> 
>   #define elf32_cr16_link_hash_traverse(table, func, info)    \
>     elf_link_hash_traverse (&(table)->root,                   \
>      (bfd_boolean (*) (struct elf_link_hash_entry *, void *)) (func), \
>      (info))
> 
> This applies to the other uses of PTR in the patch as well...

OK. I will correct it.

  >    +            /* Add or subtract the offset value.  */
> 
> Similarly:

OK. I will correct typos and replace C++ style comment with C style.

> Why is this section of code suppressed ?  Is it unnecessary, or
> something that is broken but which you intend to fix in the future ?  If
> the code is never going to be used then it should just be removed.

Code caused here broken the expected behavior, so it was commented for now and 
it will be fixed in future.

> You should document these new cr16 assembler operand qualifiers in
> gas/doc/c-cr16.texi.

OK. I will update the texi file.


> with object files/libraries that use the new values.  It would be better
> to just add the new R_CR16_GOT... relocs to the end of the current list
> of relocs.

OK. I will append the new relocs instead of insert.

Once again thanks for your detailed comments/suggestions. I will update and 
submit for review soon.

Thanks
Swami



More information about the Binutils mailing list