Re: [U-Boot] [PATCH 2/3] fix checkpatch errors

[Wolfgang Denk]

Dear Georg Schardt,

In message <[EMAIL PROTECTED]> you wrote:
> ---
>  include/configs/FX12MM.h |   12 ++--
>  1 files changed, 6 insertions(+), 6 deletions(-)

This file does not even exist yet.

Please fix the code in your local branch, rebase your patch, and
resubmit a cleaned up patch. 

But make sure it actually compiles.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: [EMAIL PROTECTED]
Don't you know anything? I should have thought anyone knows that  who
knows anything about anything...  - Terry Pratchett, _Soul Music_
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH 2/3] fix checkpatch errors

[Georg Schardt]

-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1
 

i tried to rebase my local testing-branch with my local master-branch,
but i always get the message that the branch is up to date.

then i merge my master-branch with "git pull . testing"  with the
testing-branch, but "git format-patch origin"  creates me a patch-file
for every commit from the testing-branch.

how can i create a single file patch with the differents between the
head of  my testing-branch and the head of the master-branch ?

regards
georg


Wolfgang Denk schrieb:
> Dear Georg Schardt,
>
> In message <[EMAIL PROTECTED]> you wrote:
>> ---
>>  include/configs/FX12MM.h |   12 ++--
>>  1 files changed, 6 insertions(+), 6 deletions(-)
>
> This file does not even exist yet.
>
> Please fix the code in your local branch, rebase your patch, and
> resubmit a cleaned up patch.
>
> But make sure it actually compiles.
>
> Best regards,
>
> Wolfgang Denk
>

-BEGIN PGP SIGNATURE-
Version: GnuPG v1.4.9 (MingW32)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org
 
iEYEARECAAYFAkjCjV4ACgkQUicxT/v10ZveRQCfSUUfqVC90k/C8OaW+NS3LAc+
OVYAnA62F9hbvPI8BrhBbsau2aCBV+gK
=OVKI
-END PGP SIGNATURE-

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH 2/3] fix checkpatch errors

[Wolfgang Denk]

Dear Georg Schardt,

In message <[EMAIL PROTECTED]> you wrote:
> 
> i tried to rebase my local testing-branch with my local master-branch,
> but i always get the message that the branch is up to date.

Which exact commands are you using? 


Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: [EMAIL PROTECTED]
Very ugly or very beautiful women should be flattered on their
understanding, and mediocre ones on their beauty.
   -- Philip Earl of Chesterfield
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH 2/3] fix checkpatch errors

[Georg Schardt]

Dear Wolfgang Denk,

thank you for the help. I use the following commands:

- create my own testing branch: "git branch testing"
- switch to this branch: "git checkout testing"
- copy/modify files
- add the new files with "git add board/xilinx/fx12mm/"
- commit the changes with "git commit -a"
- create a patch with "git format-patch origin"
- check this patch with checkpatch.pl
- fix the errors, commit again, create patch again get 2 patchfiles
- then i try "git rebase master" and get the message "Current branch
master is up to date"

thanks
georg


Wolfgang Denk wrote:
> Dear Georg Schardt,
>
> In message <[EMAIL PROTECTED]> you wrote:
>   
>> i tried to rebase my local testing-branch with my local master-branch,
>> but i always get the message that the branch is up to date.
>> 
>
> Which exact commands are you using? 
>
>
> Best regards,
>
> Wolfgang Denk
>
>   


___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH 2/3] fix checkpatch errors

[Wolfgang Denk]

Dear Georg Schardt,

In message <[EMAIL PROTECTED]> you wrote:
> 
> - create my own testing branch: "git branch testing"
> - switch to this branch: "git checkout testing"
> - copy/modify files
> - add the new files with "git add board/xilinx/fx12mm/"
> - commit the changes with "git commit -a"
> - create a patch with "git format-patch origin"
> - check this patch with checkpatch.pl
> - fix the errors, commit again, create patch again get 2 patchfiles

This all looks good so far

> - then i try "git rebase master" and get the message "Current branch
> master is up to date"

Hm... the "current branch master" makes be believe that you might have
checked out the master branch now? You should still have the "testing"
branch checked out.

Also note, that when you want to combine the two commits into one for
submission, you have to use "git-rebase -i master" to run in
interactive mode.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: [EMAIL PROTECTED]
"Have you lived in this village all your life?""No, not yet."
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH 2/3] fix checkpatch errors

[Georg Schardt]

Hi again,
> This all looks good so far
>
>   
>> - then i try "git rebase master" and get the message "Current branch
>> master is up to date"
>> 
>
> Hm... the "current branch master" makes be believe that you might have
> checked out the master branch now? You should still have the "testing"
> branch checked out.
>   

you are right, sorry. but with testing checkout there is no different

[EMAIL PROTECTED]:~/embedded/u-boot$ git branch
  i.MX31
  lwmon5
  master
  origin
* testing
  [EMAIL PROTECTED]

[EMAIL PROTECTED]:~/embedded/u-boot$ git-rebase master
Current branch testing is up to date.

> Also note, that when you want to combine the two commits into one for
> submission, you have to use "git-rebase -i master" to run in
> interactive mode.
>
>   
Huuh, i have no option -i available. i use git 1.4.4.4. *argl*

regards
georg

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH 2/3] fix checkpatch errors

[Wolfgang Denk]

Dear JerryVanBaren,

In message <[EMAIL PROTECTED]> you wrote:
> Georg Schardt wrote:
...
> > -#define RM_SYSTEMACE_CMDS   | CFG_CMD_FAT
> > +#define RM_SYSTEMACE_CMDS   ( | CFG_CMD_FAT )
...

> Philosophical question: is it better to put silly parenthesis around 
> #defines to make checkpatch shut up or to accept that checkpatch isn't 
> perfect and let it bitch about things that were done intentionally and 
> make sense per their usage?

Well, the most important thin is actually that the code is correct,
and the second important thing is that it can be compiled.

With above definitions of "( | CFG_CMD_FAT )" and  similar,  I  think
that  the code would NOT compile; instead, I expect GCC to issue some
"error: expected expression before '|' token"  error  messages  (most
probably  followed by some other "error: called object 'foo' is not a
function" erros.

> (Yes, I see the first one already had () and the change is just fixing 
> the spacing.)

No matter if it fixes spacing or not - it breaks the code.


> I'm serious about this question: in my day job I see a lot of mechanical 

OK, here is a serious anser: no. Such changes as suggested above make
zero sense even if they don't effect correctness of code.

See my previous discussion with Guennadi - changing all "return (1);"
into "return 1;" makes no sense to me when it's done just to silence
checkpatch.pl.

> It is WRONG to let our tools rule us.[1]

Agreed 100%.


Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: [EMAIL PROTECTED]
Do not underestimate the value of print statements for debugging.
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH 2/3] fix checkpatch errors

[Haavard Skinnemoen]

JerryVanBaren <[EMAIL PROTECTED]> wrote:
> Georg Schardt wrote:
> > ---
> >  include/configs/FX12MM.h |   12 ++--
> >  1 files changed, 6 insertions(+), 6 deletions(-)
> > 
> > diff --git a/include/configs/FX12MM.h b/include/configs/FX12MM.h
> > index b47e403..8b8d41c 100644
> > --- a/include/configs/FX12MM.h
> > +++ b/include/configs/FX12MM.h
> > @@ -15,28 +15,28 @@
> >  #define CONFIG_DOS_PARTITION1
> >  #define CFG_SYSTEMACE_BASE  XPAR_SYSACE_0_BASEADDR
> >  #define CFG_SYSTEMACE_WIDTH XPAR_SYSACE_0_MEM_WIDTH
> > -#define ADD_SYSTEMACE_CMDS  (| CFG_CMD_FAT)
> > +#define ADD_SYSTEMACE_CMDS  ( | CFG_CMD_FAT )
> >  #define RM_SYSTEMACE_CMDS
> >  #else
> >  #define ADD_SYSTEMACE_CMDS
> > -#define RM_SYSTEMACE_CMDS   | CFG_CMD_FAT
> > +#define RM_SYSTEMACE_CMDS   ( | CFG_CMD_FAT )
> 
> Dear List,
> 
> Philosophical question: is it better to put silly parenthesis around 
> #defines to make checkpatch shut up or to accept that checkpatch isn't 
> perfect and let it bitch about things that were done intentionally and 
> make sense per their usage?

In this particular case, I'd recommend not starting the macro with a
binary operator in the first place. That's just _wrong_.

Better define ADD_SYSTEMACE_CMDS etc. as 0 if no flags are to be
added/removed, and add the operator at the callsite.

> (Yes, I see the first one already had () and the change is just fixing 
> the spacing.)

The checkpatch author probably never considered that anyone would
actually define a macro beginning with a binary operator, so it's no
surprise that it comes up with strange suggestions.

That said, putting parentheses around macro contents is a good rule in
general since it might avoid quite a few surprises. E.g.

#define FOO -1

bar = 2FOO; /* should give a compile error, but doesn't */

> I'm serious about this question: in my day job I see a lot of mechanical 
> praying to the god of miserableC (MISRA-C) adding a HUGE amount of 
> unnecessary syntax noise such that it becomes hard to read the code 
> because of all the noise.  I've had people at work ask me why "we" 
> cannot write code that is as easy to understand as the linux kernel. 
> The answer is simple: "we" are slavishly and mechanically following the 
> god of "if it was good practice somewhere, sometime, it must always be a 
> good practice" and not applying good engineering judgment and experience.

I'm not particularly familiar with MISRA-C, but from what I've seen of
it, it's a particularly horrible example of following rules just for
the sake of the rules.

> It is WRONG to let our tools rule us.[1]

Agreed.

Haavard
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH 2/3] fix checkpatch errors

[JerryVanBaren]

Georg Schardt wrote:
> ---
>  include/configs/FX12MM.h |   12 ++--
>  1 files changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/include/configs/FX12MM.h b/include/configs/FX12MM.h
> index b47e403..8b8d41c 100644
> --- a/include/configs/FX12MM.h
> +++ b/include/configs/FX12MM.h
> @@ -15,28 +15,28 @@
>  #define CONFIG_DOS_PARTITION1
>  #define CFG_SYSTEMACE_BASE  XPAR_SYSACE_0_BASEADDR
>  #define CFG_SYSTEMACE_WIDTH XPAR_SYSACE_0_MEM_WIDTH
> -#define ADD_SYSTEMACE_CMDS  (| CFG_CMD_FAT)
> +#define ADD_SYSTEMACE_CMDS  ( | CFG_CMD_FAT )
>  #define RM_SYSTEMACE_CMDS
>  #else
>  #define ADD_SYSTEMACE_CMDS
> -#define RM_SYSTEMACE_CMDS   | CFG_CMD_FAT
> +#define RM_SYSTEMACE_CMDS   ( | CFG_CMD_FAT )

Dear List,

Philosophical question: is it better to put silly parenthesis around 
#defines to make checkpatch shut up or to accept that checkpatch isn't 
perfect and let it bitch about things that were done intentionally and 
make sense per their usage?

(Yes, I see the first one already had () and the change is just fixing 
the spacing.)

I'm serious about this question: in my day job I see a lot of mechanical 
praying to the god of miserableC (MISRA-C) adding a HUGE amount of 
unnecessary syntax noise such that it becomes hard to read the code 
because of all the noise.  I've had people at work ask me why "we" 
cannot write code that is as easy to understand as the linux kernel. 
The answer is simple: "we" are slavishly and mechanically following the 
god of "if it was good practice somewhere, sometime, it must always be a 
good practice" and not applying good engineering judgment and experience.

It is WRONG to let our tools rule us.[1]

Thanks for letting me unload,
gvb

[1] It took four movies to kill the Terminators, and now I see we still 
haven't succeeded.
   

P.S. In my day job I've seen way too much lot of...
#define FOOBAR_THREE(3)

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH 2/3] fix checkpatch errors

[Hugo Villeneuve]

git diff ${GITDIFF_OPTS} ${commit} >> [EMAIL PROTECTED] wrote:
> Dear Georg Schardt,
> 
> In message <[EMAIL PROTECTED]> you wrote:
>> 
>> - create my own testing branch: "git branch testing"
>> - switch to this branch: "git checkout testing"
>> - copy/modify files
>> - add the new files with "git add board/xilinx/fx12mm/"
>> - commit the changes with "git commit -a"
>> - create a patch with "git format-patch origin"
>> - check this patch with checkpatch.pl
>> - fix the errors, commit again, create patch again get 2 patchfiles

I´m not an expert with GIT, but I would suggest the following change to avoid 
unnecessary commits:

1.  Create my own testing branch: "git branch testing"
2.  Switch to this branch: "git checkout testing"
3.Copy/modify files
4.Create temporary patch from local changes: "git diff > temporary.patch"
5.Check this patch with checkpatch.pl
6.  If errors found: go back to step 3, else go to step 7
7.  Add the new/modified files with "git add board/xilinx/fx12mm/"
8.  Commit the changes with "git commit -a"
9.  Create a patch with "git format-patch origin"
10. Check this patch with checkpatch.pl

Hugo V.

Hugo Villeneuve
Hardware developer | Concepteur matériel
Lyrtech
Phone/Tél. : (1) (418) 877-4644 #2395
Toll-free/Sans frais - Canada & USA : (1) (888) 922-4644 #2395
Fax/Téléc. : (1) (418) 877-7710
www.lyrtech.com
Infinite possibilities...TM
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot