* Re: [ibm-acpi-devel] [PATCH 8/14] drivers/platform/x86/thinkpad_acpi.c: delete double assignment
2010-10-27 1:09 ` Henrique de Moraes Holschuh
@ 2010-10-27 1:24 ` Henrique de Moraes Holschuh
2010-10-27 3:54 ` Dan Carpenter
2010-10-27 5:03 ` Julia Lawall
2 siblings, 0 replies; 7+ messages in thread
From: Henrique de Moraes Holschuh @ 2010-10-27 1:24 UTC (permalink / raw)
To: Julia Lawall
Cc: kernel-janitors, linux-kernel, platform-driver-x86,
ibm-acpi-devel, Matthew Garrett
On Tue, 26 Oct 2010, Henrique de Moraes Holschuh wrote:
> On Tue, 26 Oct 2010, Julia Lawall wrote:
> > From: Julia Lawall <julia@diku.dk>
> >
> > Delete successive assignments to the same location.
>
> There are often results of mismerges or other assorted screw ups, often
> done by the maintainer itself (e.g. me in thinkpad-acpi's case) during
> development.
Which was the case, and the error was just a duplicate line indeed and not
anything more damaging in one of my commits done a few years ago.
So,
Acked-by: Henrique de Moraes Holschuh <hmh@hmh.eng.br>
--
"One disk to rule them all, One disk to find them. One disk to bring
them all and in the darkness grind them. In the Land of Redmond
where the shadows lie." -- The Silicon Valley Tarot
Henrique Holschuh
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 8/14] drivers/platform/x86/thinkpad_acpi.c: delete double assignment
2010-10-27 1:09 ` Henrique de Moraes Holschuh
2010-10-27 1:24 ` [ibm-acpi-devel] " Henrique de Moraes Holschuh
@ 2010-10-27 3:54 ` Dan Carpenter
2010-10-27 21:42 ` Henrique de Moraes Holschuh
2010-10-27 5:03 ` Julia Lawall
2 siblings, 1 reply; 7+ messages in thread
From: Dan Carpenter @ 2010-10-27 3:54 UTC (permalink / raw)
To: Henrique de Moraes Holschuh
Cc: Julia Lawall, Henrique de Moraes Holschuh, kernel-janitors,
Matthew Garrett, ibm-acpi-devel, platform-driver-x86,
linux-kernel
On Tue, Oct 26, 2010 at 11:09:08PM -0200, Henrique de Moraes Holschuh wrote:
> On Tue, 26 Oct 2010, Julia Lawall wrote:
> > From: Julia Lawall <julia@diku.dk>
> >
> > Delete successive assignments to the same location.
>
> There are often results of mismerges or other assorted screw ups, often
> done by the maintainer itself (e.g. me in thinkpad-acpi's case) during
> development.
>
> The patch is correct, and I thank you for the head's up. But let me
> track down what caused it first, hmm? I also highly recommend that this
> should be done in all cases you find, instead of just blindly fixing the
> assignment.
I've reviewed the entire patchset per your suggestion and Julia
obviously didn't do it blindly. It all looks good. The patcheset fixes
a number of bugs.
regards,
dan carpenter
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 8/14] drivers/platform/x86/thinkpad_acpi.c: delete double assignment
2010-10-27 3:54 ` Dan Carpenter
@ 2010-10-27 21:42 ` Henrique de Moraes Holschuh
0 siblings, 0 replies; 7+ messages in thread
From: Henrique de Moraes Holschuh @ 2010-10-27 21:42 UTC (permalink / raw)
To: Dan Carpenter, Julia Lawall, Henrique de Moraes Holschuh,
kernel-janitors, Matthew Garrett
On Wed, 27 Oct 2010, Dan Carpenter wrote:
> On Tue, Oct 26, 2010 at 11:09:08PM -0200, Henrique de Moraes Holschuh wrote:
> > On Tue, 26 Oct 2010, Julia Lawall wrote:
> > > From: Julia Lawall <julia@diku.dk>
> > >
> > > Delete successive assignments to the same location.
> >
> > There are often results of mismerges or other assorted screw ups, often
> > done by the maintainer itself (e.g. me in thinkpad-acpi's case) during
> > development.
> >
> > The patch is correct, and I thank you for the head's up. But let me
> > track down what caused it first, hmm? I also highly recommend that this
> > should be done in all cases you find, instead of just blindly fixing the
> > assignment.
>
> I've reviewed the entire patchset per your suggestion and Julia
> obviously didn't do it blindly. It all looks good. The patcheset fixes
> a number of bugs.
Thank you, Dan and Julia.
--
"One disk to rule them all, One disk to find them. One disk to bring
them all and in the darkness grind them. In the Land of Redmond
where the shadows lie." -- The Silicon Valley Tarot
Henrique Holschuh
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 8/14] drivers/platform/x86/thinkpad_acpi.c: delete double assignment
2010-10-27 1:09 ` Henrique de Moraes Holschuh
2010-10-27 1:24 ` [ibm-acpi-devel] " Henrique de Moraes Holschuh
2010-10-27 3:54 ` Dan Carpenter
@ 2010-10-27 5:03 ` Julia Lawall
2 siblings, 0 replies; 7+ messages in thread
From: Julia Lawall @ 2010-10-27 5:03 UTC (permalink / raw)
To: Henrique de Moraes Holschuh
Cc: Henrique de Moraes Holschuh, kernel-janitors, Matthew Garrett,
ibm-acpi-devel, platform-driver-x86, linux-kernel
On Tue, 26 Oct 2010, Henrique de Moraes Holschuh wrote:
> On Tue, 26 Oct 2010, Julia Lawall wrote:
> > From: Julia Lawall <julia@diku.dk>
> >
> > Delete successive assignments to the same location.
>
> There are often results of mismerges or other assorted screw ups, often
> done by the maintainer itself (e.g. me in thinkpad-acpi's case) during
> development.
>
> The patch is correct, and I thank you for the head's up. But let me
> track down what caused it first, hmm? I also highly recommend that this
> should be done in all cases you find, instead of just blindly fixing the
> assignment. I suggest using git --blame to track down what added the
> duplicated assignments, and check if it looks sane...
>
> It is often a VERY BAD IDEA to remove such markers of potential
> brokennes without checking out if they're actually helpfully trying to
> warn you of worse badness :p
>
> But hey, maybe you've already done that. If you did, I apologise for
> preaching to the choir.
I didn't look for the original source of the problem, except in the case
of [PATCH 6/14] arch/mips/pmc-sierra/yosemite/setup.c where I found it
already in the pre-git era. Thanks fo rthe suggestion. On the other
hand, I was very selective about which code I changed. In particular I
left a number of cases like:
x = NULL; // or 0 etc
x = foo(...); // ie a NULL returning function
I figured that this isn't hurting anything, and perhaps it is informative
about what kind of value foo might return.
julia
^ permalink raw reply [flat|nested] 7+ messages in thread