Re: [U-Boot] [PATCH 2/3] fix checkpatch errors
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
Re: [U-Boot] [PATCH 2/3] fix checkpatch errors
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
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
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
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
[U-Boot] [PATCH 2/3] fix checkpatch errors
--- 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 ) #endif #define CFG_ENV_IS_IN_FLASH 1 /* environment is in FLASH */ -#define ADD_FLASH_CMDS | CFG_CMD_FLASH +#define ADD_FLASH_CMDS ( | CFG_CMD_FLASH ) #define RM_FLASH_CMDS #ifdef XPAR_IIC_0_DEVICE_ID -#if ! defined(CFG_ENV_IS_IN_FLASH) +#if !defined(CFG_ENV_IS_IN_FLASH) #define CFG_ENV_IS_IN_EEPROM1 /* environment is in IIC EEPROM */ #endif -#define ADD_IIC_CMDS| CFG_CMD_I2C +#define ADD_IIC_CMDS( | CFG_CMD_I2C ) #define RM_IIC_CMDS #else #define ADD_IIC_CMDS -#define RM_IIC_CMDS | CFG_CMD_I2C +#define RM_IIC_CMDS ( | CFG_CMD_I2C ) #endif -- 1.4.4.4 ___ 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
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
-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
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
