This is the mail archive of the binutils@sources.redhat.com mailing list for the binutils 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 for avr port of binutils [adiw, sbiw, ldd with more functionality]


Hi Klaus,

Here is the patch for approval.

[Note - your first email did get through, as did the second].


I am sorry but I am not going to approve the patch in its current state. There are several problems with it which I hope you will take into consideration and then choose to submit a revised version:

* The file bfd-in2.h is an automatically generated file. If you need to patch it, you should also patch the source file(s) that are used to create it. In this particular case the new relocs that you created needed to be added to the bfd/reloc.c file.

* You used C++ style comments in a few places in the code. Although GCC allows this, other compiler might not, and besides we are trying to get to the state where we can compile all of binutils with the -Wall -Werror flags set.

* We are now using ISO-C90 style C formatting in binutils, and although there are quite a lot of files that have not yet been converted over, we still ike to encourage any new patches to make use of this standard. Thus for example the PARAMS() macro is now no longer needed when declaring function prototypes, and function declarations can include their arguments inside the parentheses.

* The patch needlessly inserted whitespace in various places and did not always follow the GNU Coding convention.

* It is helpful if you can generate patches using the -c switch to diff. The extra context makes the patch much easier to read.

* When adding a new feature it is always a good idea to include a new entry in the relevent testsuite to make sure that the feature works, and continues to work in the future.

* You should always include a set of entries for the relevent ChangeLog files annotating which files you have changed and why. Note - for convenince we ask that these entries just be submitted as plain text, not a diff, as they usually have to be applied by hand.

Cheers
  Nick


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