Hi!

Thank you for the advice, I will do my best to live up to them :)

I'm new to git as well, I tried making the patch you requested:

Cheers,
  Ákos

>From 88a430100be4d9685863aa2912173ddc5b83e92f Mon Sep 17 00:00:00 2001
From: Akos <akos@FM12BQ.(none)>
Date: Sun, 25 Sep 2011 23:55:36 +0200
Subject: [PATCH] Fixed two warnings for uninitialized variables
generating build errors

Two retvals in the functions oocd_swd_transport_init and
mem_ap_read_buf_u32 have not had their values initialized
and generated a warning, which cause the build to fail.
Fixed by initializing these variables to ERROR_OK
---
 src/target/arm_adi_v5.c  |    2 +-
 src/transport/swd_core.c |    2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/target/arm_adi_v5.c b/src/target/arm_adi_v5.c
index f529446..1454007 100644
--- a/src/target/arm_adi_v5.c
+++ b/src/target/arm_adi_v5.c
@@ -696,7 +696,7 @@ int mem_ap_read_buf_u32(struct adiv5_dap *dap,
uint8_t *buffer,
 //     uint32_t invalue, adr = address;
 //     uint8_t* pBuffer = buffer;

-       int i, retval;
+       int i, retval = ERROR_OK;
        uint32_t invalue;
        //count >>= 2;
 //     wcount = count;
diff --git a/src/transport/swd_core.c b/src/transport/swd_core.c
index 383199f..36191fb 100644
--- a/src/transport/swd_core.c
+++ b/src/transport/swd_core.c
@@ -125,7 +125,7 @@ int oocd_swd_run(struct adiv5_dap *dap){
 // TODO: We are operating on global interface pointer, change it into
function parameter asap.
 int oocd_swd_transport_init(struct command_context *ctx){
        LOG_DEBUG("entering function...");
-       int retval, *idcode;
+       int *idcode, retval = ERROR_OK;

        struct target *target = get_current_target(ctx);
        struct arm *arm = target_to_arm(target);
-- 
1.7.1


On 25 September 2011 23:15, Peter Stuge <[email protected]> wrote:
> Hi Akos,
>
> Akos Vandra wrote:
>> Bingo, I am working on Ubuntu 10.10, x64, gcc 4.4.5 (shipped with ubuntu).
>
> 10.10 is soon one year old, which can be a long time in open source.
>
> Also keep in mind that pretty much every major distribution is
> applying significant amounts of patches in their toolchain packages
> and possibly also system libraries.
>
> They intend this to make things better of course, but it can backfire
> and make the toolchain unusable sometimes. Keep this in mind if you
> encounter a weird build issue of any package.
>
> From many years of use of gcc and binutils I would recommend to use
> a vanilla toolchain, or at the very least very aggressively pulling
> toolchain updates from your distribution.
>
>
>> I have been getting some warnings (treated as errors) due to some
>> retvals didn't have initial value set, but I set them to ERROR_OK,
>> and now it passes those points.
>
> Please send patch(es) for these to the mailing list. Thanks!
>
>
>> It seems like uint32_t is not defined, even though it should be, as
>> stdint.h is included.
>
> stdint.h is included only #ifdef HAVE_STDINT_H. Did you verify with
> gcc -E or at least in config.h that HAVE_STDINT_H is defined?
>
>
>> Funny thing, if I set the first occurance of uint32_t to unsigned
>> int, the other ones build fine, which is kinda wierd.
>
> It is not funny at all. I find it typical of a toolchain issue. It is
> bullshit that wastes precious developer time. :\
>
>
>> Also, should we spamming the developer list,
>
> Certainly yes. The purpose of the developer list is to carry
> developer discussion.
>
>
>> or rather continue this discussion personally?
>
> Absolutely not.
>
>
>> I'm not familiar with opensource development *at all*,
>
> Welcome to open source! It can be a wonderful environment of
> cooperation and synergy when things align well. It can also be a
> challenging environment of conflicts and complaints when they don't.
>
> Overall I personally find it to be more than worth the trouble, and
> I think it leads to better software.
>
>
>> so I'm not familiar with list policies either.
>
> Again, this is a developer list whose sole purpose is for developer
> communication. Direct communication should be an exception.
> (Sometimes it's the right thing of course, just usually it is not.)
>
> Please study git and work on becoming fluent with it if you are not
> already. It is a great tool that supports development in very many
> ways. Beyond simply keeping track of what happened in the code git
> when used well allows incredibly efficient review and movement of
> code between developers in a project. I can recommend the
> http://progit.org/ book, but start out by just playing with it a
> little. Make some repositories with some commits and get the basic
> workflow going. Learning the rest is usually easy. I also recommend
> the #git IRC channel on freenode, for getting live help.
>
>
> One use for developer mailing lists is to distribute proposed
> patches, so that they can be reviewed and then included into the
> public source tree.
>
> To make the review step quick and thus make it somewhat likely to
> actually happen, it is *critical* that the patch is what I call
> "clean". Maybe all this is obvious to you already, in that case
> please skip to bottom. The individual points of a clean patch are:
>
>
> * Write a top quality commit message, technically and logically
>
> In git repositories it is useful to follow a certain commit message
> format: (Line numbers added to the left)
>
> 1. Description of change, prefer <=60 chars, absmax 74 or even 72 chars
> 2.
> 3. Third line and onward hold longer description of change
> 4. each line absmax 74 chars
>
> The first line is used in list views, where one commit is listed per
> line. This makes it important to have a short and sweet description
> of the change in the first line. The second line is always left
> blank. The third line and any number of following lines can and
> should include background on this part of the code and discuss why
> this change was made the way it was done instead of some other more
> obvious way. Ie. document the research that went into this particular
> change of the code. Including links to web pages with relevant
> information is helpful, as is links to mailing list messages e.g. on
> http://marc.info/ which has nice short message links. (Only the m=
> URL parameter is required for links to a message.)
>
> The line length restrictions are in part due to use of 80 char wide
> terminals, but more importantly they are due to how patches are sent
> and refered to in email. I.e. even if in an open source project no
> active developer is using 80 char wide terminals there may be
> participants on the mailing list who benefit from the shorter lines,
> in which case it makes sense to keep them short. Also, web based
> repository viewers tend to not want or be able to present very long
> first line descriptions.
>
> As for the logical quality, it is important to write the first line
> description of the change so that it makes sense for someone who
> knows nothing at all about the code, since this is used in list
> views, and since the background for this code and for why this change
> was done the way it was done comes only in the later lines, which may
> not be available from where that list view is. On a web page of
> course it is, but a list of commits can easily appear in an email,
> which is read by someone offline on an airplane. A description
> refering to code they don't have and don't know is useless. Keep it
> high level, clear and simple. Writing this one line is not
> neccessarily easy.
>
> Writing the third line and onwards is usually easier. Before creating
> this commit you started at some point, faced with a task, a problem
> or even a bug ticket, and you then learned some things in order to
> make the committed code change. Basically you want to make sure that
> a developer doing review also learns the same things, so that they
> are able to adequatly review your conclusions, without having to
> duplicate the time you spent on learning how and why to make the
> change. A bonus for reviewers is that they learn a lot from code
> written by others.
>
>
> * Change only one thing per commit
>
> From the above discussion on commit message it is clear that in order
> to write a useful commit message it is critical that one commit only
> includes one logical change. Otherwise it's impossible to write a one
> line summary.
>
> So what's a "thing" - well any logical change, small or big. It can
> be about renaming variables in order to prepare for a later commit
> which does code refactoring. It can also be one bug fix which touches
> a lot of different places in the code. The developer creating the
> commit decides on what is the thing, and indeed one part of review is
> for other developers to give feedback on how their view of this thing
> align. Sometimes this will lead to reworking the patch to solve the
> same problem with other logical steps in the code.
>
> Besides being common sense, the one thing per commit rule is also
> technically important, e.g. in order for the git bisect feature to be
> meaningful, and also for being able to later revert a logical change
> which has turned out to be a bad idea after all.
>
>
> * Adhere strictly to code style where you work
>
> A commit can change lots of different places in the code and for
> historical reasons it's totally possible that they use different code
> style. Per the previous point about one thing per commit any code
> style changes should be a separate commit, even if it is tempting
> also for me to clean up code when I am anyway working in that area.
> This is fine, but the critical point is to not commit them together.
> git add -p will allow to post process changes in a file, before they
> get committed, so that you can commit style change/cleanup
> separately. In general, reduce code style changes to an absolute
> minimum. Not only are style changes very demanding to review (since
> they usually change large amounts of code) but style changes also
> clutter repository history, which (and this is important) makes it a
> lot more work to go back and look at the history of a piece of code,
> e.g. to compare what the code did a year ago when it worked, with
> what it does now when it has regressed. A big style change of the
> code adds enormous overhead to fixing this problem. Get style right
> the first time. Hopefully review will catch problems, but be careful
> to try to avoid code style issues.
>
>
> * Do review yourself before creating a commit
>
> With git it is amazingly easy to perform review of the changes one
> has made, using git diff. Always use this to look for all of the
> above points, before you create the commit. When working in the
> source files it is easy to get tunnel vision and make some small
> change that gets forgotten later, when focusing on a particular
> problem. When looking at git diff output that small change is so
> obvious, and it can be taken care of without requiring a round trip
> to any other developer.
>
>
> * Do review yourself before sending commits to anyone else
>
> Especially for long gnarly sessions leading to a commit it's possible
> to still miss something. Reviewing the changes before posting is so
> easy because they are available as commits in the local repository.
> git log -p shows each commit with commit message and all changes to
> the code, and assuming your terminal supports it the output will even
> be colour coded. Do this before you send a patch to the mailing list
> (e.g. using git send-email) for review by others, again saving time
> for others and reducing round trips.
>
>
> This might seem like an aweful lot to keep in mind just for working
> on some code, but I hope that I was able to give good reasons for
> each point and if they are summarized, actually it doesn't look
> like such a mouthful anymore:
>
> * Write a top quality commit message, technically and logically
> * Change only one thing per commit
> * Adhere strictly to code style where you work
> * Do review yourself before creating a commit
> * Do review yourself before sending commits to anyone else
>
>
> I hope this makes sense. Please feel free to ask any questions.
>
>
> //Peter
> _______________________________________________
> Openocd-development mailing list
> [email protected]
> https://lists.berlios.de/mailman/listinfo/openocd-development
>
_______________________________________________
Openocd-development mailing list
[email protected]
https://lists.berlios.de/mailman/listinfo/openocd-development

Reply via email to