[PATCH] blackfin/sram: use 'unsigned long' for irqflags

Previous thread: [PATCH 1/3] binfmt_elf_fdpic: Support auxvec base platform string. by Paul Mundt on Wednesday, August 6, 2008 - 3:35 am. (6 messages)

Next thread: Re: 2.6.27rc1 cannot boot more than 8CPUs by Jeff Chua on Wednesday, August 6, 2008 - 4:09 am. (13 messages)
From: Vegard Nossum
Date: Wednesday, August 6, 2008 - 3:58 am

(Use a wider diff context such as -U10 after the patch has been applied to
see the actual uses of the variables which we change.)

From 3ef36948a88a968eec1b09859aa251dc6727df4e Mon Sep 17 00:00:00 2001
From: Vegard Nossum <vegard.nossum@gmail.com>
Date: Wed, 6 Aug 2008 12:00:23 +0200
Subject: [PATCH] blackfin/sram: use 'unsigned long' for irqflags

Using just 'unsigned' will make flags an unsigned int. While this is
arguably not an error on blackfin where sizeof(int) == sizeof(long),
the patch is still justified on the grounds of principle.

The patch was generated using the Coccinelle semantic patch framework.

Cc: Julia Lawall <julia@diku.dk>
Cc: Alexey Dobriyan <adobriyan@gmail.com>
Signed-off-by: Vegard Nossum <vegard.nossum@gmail.com>
---
 arch/blackfin/mm/blackfin_sram.c |   22 +++++++++++-----------
 1 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/arch/blackfin/mm/blackfin_sram.c b/arch/blackfin/mm/blackfin_sram.c
index 5af3c31..9bc6aed 100644
--- a/arch/blackfin/mm/blackfin_sram.c
+++ b/arch/blackfin/mm/blackfin_sram.c
@@ -379,7 +379,7 @@ EXPORT_SYMBOL(sram_free);
 
 void *l1_data_A_sram_alloc(size_t size)
 {
-	unsigned flags;
+	unsigned long flags;
 	void *addr = NULL;
 
 	/* add mutex operation */
@@ -402,7 +402,7 @@ EXPORT_SYMBOL(l1_data_A_sram_alloc);
 
 int l1_data_A_sram_free(const void *addr)
 {
-	unsigned flags;
+	unsigned long flags;
 	int ret;
 
 	/* add mutex operation */
@@ -425,7 +425,7 @@ EXPORT_SYMBOL(l1_data_A_sram_free);
 void *l1_data_B_sram_alloc(size_t size)
 {
 #if L1_DATA_B_LENGTH != 0
-	unsigned flags;
+	unsigned long flags;
 	void *addr;
 
 	/* add mutex operation */
@@ -450,7 +450,7 @@ EXPORT_SYMBOL(l1_data_B_sram_alloc);
 int l1_data_B_sram_free(const void *addr)
 {
 #if L1_DATA_B_LENGTH != 0
-	unsigned flags;
+	unsigned long flags;
 	int ret;
 
 	/* add mutex operation */
@@ -504,7 +504,7 @@ EXPORT_SYMBOL(l1_data_sram_free);
 void *l1_inst_sram_alloc(size_t size)
 {
 #if L1_CODE_LENGTH != ...
From: Mike Frysinger
Date: Wednesday, August 6, 2008 - 4:05 am

should be CC Bryan Wu as well ...
-mike
--

From: Vegard Nossum
Date: Wednesday, August 6, 2008 - 4:28 am

Hi,


Hm? I'm sorry, I don't understand what you mean by that. Do you think

Oh right. I believed that Bryan Wu is the maintainer who would
eventually apply the patch, therefore I put him as a direct recipient
of the e-mail (To-field) and that he would add his Signed-off-by line
which would make the Cc-line redundant. I guess it makes some sense to
also list the maintainer in the patch Ccs since the patch itself may
be separate from the raw e-mail (e.g. in mail archives, which
generally don't display To: or Cc: fields). I didn't realize this
before.

Thanks.


Vegard

-- 
"The animistic metaphor of the bug that maliciously sneaked in while
the programmer was not looking is intellectually dishonest as it
disguises that the error is the programmer's own creation."
	-- E. W. Dijkstra, EWD1036
--

From: Mike Frysinger
Date: Wednesday, August 6, 2008 - 5:14 am

*shrug* ... i dont see other patches with things like:
The patch was generated with git.
The patch was generated with eclipse.
The patch was generated with emacs.
etc...

we dont generally list all of the tools in the log message that was
used in *creating* a patch since it doesnt really add any value when
looking back historically at changes.
-mike
--

From: Mike Frysinger
Date: Wednesday, August 6, 2008 - 5:17 am

although, such information may be fine in the region after --- where
the diffstat normally shows up as that will not go into the log ...
-mike
--

From: Julia Lawall
Date: Wednesday, August 6, 2008 - 5:21 am

But with git, emacs, eclipse etc, the person is still making the change by 
hand.  The comment is more like "this patch was generated with perl" or 
"this patch was generated with sed", indicating that the change is 
uniform enough to be automated.  This does have some interest, 
although perhaps not for everyone.

julia
--

From: Vegard Nossum
Date: Wednesday, August 6, 2008 - 5:52 am

Hm. I agree that git/eclipse/emacs/etc. information is not very
useful. However...

For errors found with lockdep, we usually put either the lockdep
output in the commit message or say that it was found with lockdep.
Having this information in the log is useful because it also
establishes a track record for the tool which was used to discover/fix
the error.

Arguably, the semantic patch itself should be present in the log as
well. There are different practices here, but in this case, the patch
was quite long, and has already been included in a pending commit. Now
others may use the same semantic patch (or a variation of it) and
possibly find more "bad" code (possibly introduced after the semantic
patch was first applied!).

It seems that the practice of including a reference to the tool is
already widespread, though, for example:

    $ git log v2.6.26-rc2 | grep -ci 'semantic patch'
    72

    $ git log v2.6.26-rc2 | grep -ci coverity
    495

etc.


Vegard

PS: This is a bit of a sore spot for me having been directly involved
with the development of another such tool (kmemcheck). I will always
ask for kmemcheck to be credited in the log whenever it is used to
find and fix genuine programmer errors, simply because that's the best
way to measure its usefulness :-)

-- 
"The animistic metaphor of the bug that maliciously sneaked in while
the programmer was not looking is intellectually dishonest as it
disguises that the error is the programmer's own creation."
	-- E. W. Dijkstra, EWD1036
--

From: Mike Frysinger
Date: Wednesday, August 6, 2008 - 10:34 am

that's reasonable to include the actual source (semantic patch) that
triggered the resulting change.
-mike
--

From: Mike Frysinger
Date: Friday, August 22, 2008 - 6:44 am

make sure to grab this Bryan ?
-mike
--

From: Bryan Wu
Date: Tuesday, August 26, 2008 - 11:56 pm

Sorry for the delay, guys. I'm just back from my vacation.

I will apply this patch to our svn first and then push it to mainline later.

Thanks
-Bryan
--

Previous thread: [PATCH 1/3] binfmt_elf_fdpic: Support auxvec base platform string. by Paul Mundt on Wednesday, August 6, 2008 - 3:35 am. (6 messages)

Next thread: Re: 2.6.27rc1 cannot boot more than 8CPUs by Jeff Chua on Wednesday, August 6, 2008 - 4:09 am. (13 messages)