* [PATCH] powerpc/ipic: Fix a bounds check in ipic_set_priority()
@ 2018-12-03 14:48 Dan Carpenter
2018-12-05 3:26 ` Michael Ellerman
0 siblings, 1 reply; 11+ messages in thread
From: Dan Carpenter @ 2018-12-03 14:48 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Kim Phillips
Cc: kernel-janitors, Paul Mackerras, linuxppc-dev
The ipic_info[] array only has 95 elements so I have made the bounds
check smaller to prevent a read overflow. It was Smatch that found
this issue:
arch/powerpc/sysdev/ipic.c:784 ipic_set_priority()
error: buffer overflow 'ipic_info' 95 <= 127
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
---
I wasn't able to find any callers of this code. Maybe we removed the
last one in commit b9f0f1bb2bca ("[POWERPC] Adapt ipic driver to new
host_ops interface, add set_irq_type to set IRQ sense"). So perhaps we
should just remove it. I'm not really comfortable doing that myself,
because I don't know the code well enough and can't build test
it properly.
arch/powerpc/sysdev/ipic.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/powerpc/sysdev/ipic.c b/arch/powerpc/sysdev/ipic.c
index 6300123ce965..9d70d0687cd9 100644
--- a/arch/powerpc/sysdev/ipic.c
+++ b/arch/powerpc/sysdev/ipic.c
@@ -779,7 +779,7 @@ int ipic_set_priority(unsigned int virq, unsigned int priority)
if (priority > 7)
return -EINVAL;
- if (src > 127)
+ if (src >= ARRAY_SIZE(ipic_info))
return -EINVAL;
if (ipic_info[src].prio == 0)
return -EINVAL;
--
2.11.0
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH] powerpc/ipic: Fix a bounds check in ipic_set_priority()
2018-12-03 14:48 [PATCH] powerpc/ipic: Fix a bounds check in ipic_set_priority() Dan Carpenter
@ 2018-12-05 3:26 ` Michael Ellerman
2018-12-05 8:11 ` Julia Lawall
` (2 more replies)
0 siblings, 3 replies; 11+ messages in thread
From: Michael Ellerman @ 2018-12-05 3:26 UTC (permalink / raw)
To: Dan Carpenter, Benjamin Herrenschmidt, Kim Phillips
Cc: linuxppc-dev, kernel-janitors, Paul Mackerras
Hi Dan,
Thanks for the patch.
Dan Carpenter <dan.carpenter@oracle.com> writes:
> The ipic_info[] array only has 95 elements so I have made the bounds
> check smaller to prevent a read overflow. It was Smatch that found
> this issue:
>
> arch/powerpc/sysdev/ipic.c:784 ipic_set_priority()
> error: buffer overflow 'ipic_info' 95 <= 127
>
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> ---
> I wasn't able to find any callers of this code. Maybe we removed the
> last one in commit b9f0f1bb2bca ("[POWERPC] Adapt ipic driver to new
> host_ops interface, add set_irq_type to set IRQ sense"). So perhaps we
> should just remove it. I'm not really comfortable doing that myself,
> because I don't know the code well enough and can't build test
> it properly.
Hah wow, last usage removed in 2006!
I don't see any mention of it since then, so I'll remove it. If it
breaks something we can put it back.
Can smatch help us find things like this that are defined non-static but
never used?
cheers
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] powerpc/ipic: Fix a bounds check in ipic_set_priority()
2018-12-05 3:26 ` Michael Ellerman
@ 2018-12-05 8:11 ` Julia Lawall
2018-12-05 12:04 ` Michael Ellerman
2018-12-06 7:18 ` Christophe LEROY
2018-12-06 9:34 ` Dan Carpenter
2 siblings, 1 reply; 11+ messages in thread
From: Julia Lawall @ 2018-12-05 8:11 UTC (permalink / raw)
To: Michael Ellerman
Cc: Kim Phillips, kernel-janitors, Paul Mackerras, linuxppc-dev,
Dan Carpenter
On Wed, 5 Dec 2018, Michael Ellerman wrote:
> Hi Dan,
>
> Thanks for the patch.
>
> Dan Carpenter <dan.carpenter@oracle.com> writes:
> > The ipic_info[] array only has 95 elements so I have made the bounds
> > check smaller to prevent a read overflow. It was Smatch that found
> > this issue:
> >
> > arch/powerpc/sysdev/ipic.c:784 ipic_set_priority()
> > error: buffer overflow 'ipic_info' 95 <= 127
> >
> > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> > ---
> > I wasn't able to find any callers of this code. Maybe we removed the
> > last one in commit b9f0f1bb2bca ("[POWERPC] Adapt ipic driver to new
> > host_ops interface, add set_irq_type to set IRQ sense"). So perhaps we
> > should just remove it. I'm not really comfortable doing that myself,
> > because I don't know the code well enough and can't build test
> > it properly.
>
> Hah wow, last usage removed in 2006!
>
> I don't see any mention of it since then, so I'll remove it. If it
> breaks something we can put it back.
>
> Can smatch help us find things like this that are defined non-static but
> never used?
I wrote a Coccinelle script for this, that just uses grep. Of course the
results need checking because uses can be constructed within macros using
#.
Are things that are defined static but are never used useful to keep
around?
julia
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] powerpc/ipic: Fix a bounds check in ipic_set_priority()
2018-12-05 8:11 ` Julia Lawall
@ 2018-12-05 12:04 ` Michael Ellerman
2018-12-05 12:06 ` Julia Lawall
0 siblings, 1 reply; 11+ messages in thread
From: Michael Ellerman @ 2018-12-05 12:04 UTC (permalink / raw)
To: Julia Lawall
Cc: Kim Phillips, kernel-janitors, Paul Mackerras, linuxppc-dev,
Dan Carpenter
Julia Lawall <julia.lawall@lip6.fr> writes:
> On Wed, 5 Dec 2018, Michael Ellerman wrote:
>
>> Hi Dan,
>>
>> Thanks for the patch.
>>
>> Dan Carpenter <dan.carpenter@oracle.com> writes:
>> > The ipic_info[] array only has 95 elements so I have made the bounds
>> > check smaller to prevent a read overflow. It was Smatch that found
>> > this issue:
>> >
>> > arch/powerpc/sysdev/ipic.c:784 ipic_set_priority()
>> > error: buffer overflow 'ipic_info' 95 <= 127
>> >
>> > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
>> > ---
>> > I wasn't able to find any callers of this code. Maybe we removed the
>> > last one in commit b9f0f1bb2bca ("[POWERPC] Adapt ipic driver to new
>> > host_ops interface, add set_irq_type to set IRQ sense"). So perhaps we
>> > should just remove it. I'm not really comfortable doing that myself,
>> > because I don't know the code well enough and can't build test
>> > it properly.
>>
>> Hah wow, last usage removed in 2006!
>>
>> I don't see any mention of it since then, so I'll remove it. If it
>> breaks something we can put it back.
>>
>> Can smatch help us find things like this that are defined non-static but
>> never used?
>
> I wrote a Coccinelle script for this, that just uses grep. Of course the
> results need checking because uses can be constructed within macros using
> #.
That would be cool. I can't immediately see it in scripts/coccinelle, is
it somewhere else?
> Are things that are defined static but are never used useful to keep
> around?
No, but the compiler will usually tell us about them via -Wunused-function.
cheers
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] powerpc/ipic: Fix a bounds check in ipic_set_priority()
2018-12-05 12:04 ` Michael Ellerman
@ 2018-12-05 12:06 ` Julia Lawall
0 siblings, 0 replies; 11+ messages in thread
From: Julia Lawall @ 2018-12-05 12:06 UTC (permalink / raw)
To: Michael Ellerman
Cc: Kim Phillips, kernel-janitors, Paul Mackerras, linuxppc-dev,
Dan Carpenter
On Wed, 5 Dec 2018, Michael Ellerman wrote:
> Julia Lawall <julia.lawall@lip6.fr> writes:
> > On Wed, 5 Dec 2018, Michael Ellerman wrote:
> >
> >> Hi Dan,
> >>
> >> Thanks for the patch.
> >>
> >> Dan Carpenter <dan.carpenter@oracle.com> writes:
> >> > The ipic_info[] array only has 95 elements so I have made the bounds
> >> > check smaller to prevent a read overflow. It was Smatch that found
> >> > this issue:
> >> >
> >> > arch/powerpc/sysdev/ipic.c:784 ipic_set_priority()
> >> > error: buffer overflow 'ipic_info' 95 <= 127
> >> >
> >> > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> >> > ---
> >> > I wasn't able to find any callers of this code. Maybe we removed the
> >> > last one in commit b9f0f1bb2bca ("[POWERPC] Adapt ipic driver to new
> >> > host_ops interface, add set_irq_type to set IRQ sense"). So perhaps we
> >> > should just remove it. I'm not really comfortable doing that myself,
> >> > because I don't know the code well enough and can't build test
> >> > it properly.
> >>
> >> Hah wow, last usage removed in 2006!
> >>
> >> I don't see any mention of it since then, so I'll remove it. If it
> >> breaks something we can put it back.
> >>
> >> Can smatch help us find things like this that are defined non-static but
> >> never used?
> >
> > I wrote a Coccinelle script for this, that just uses grep. Of course the
> > results need checking because uses can be constructed within macros using
> > #.
>
> That would be cool. I can't immediately see it in scripts/coccinelle, is
> it somewhere else?
No, it needs improvement... I'll try to do something with it soon. I
don't think it is well suited to scrips/coccinelle, because it needs to
know where the kernel tree is to do the grep.
julia
>
> > Are things that are defined static but are never used useful to keep
> > around?
>
> No, but the compiler will usually tell us about them via -Wunused-function.
>
> cheers
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] powerpc/ipic: Fix a bounds check in ipic_set_priority()
2018-12-05 3:26 ` Michael Ellerman
2018-12-05 8:11 ` Julia Lawall
@ 2018-12-06 7:18 ` Christophe LEROY
2018-12-06 8:12 ` Julia Lawall
2018-12-07 2:07 ` Michael Ellerman
2018-12-06 9:34 ` Dan Carpenter
2 siblings, 2 replies; 11+ messages in thread
From: Christophe LEROY @ 2018-12-06 7:18 UTC (permalink / raw)
To: Michael Ellerman, Dan Carpenter, Benjamin Herrenschmidt, Kim Phillips
Cc: linuxppc-dev, kernel-janitors, Paul Mackerras
Le 05/12/2018 à 04:26, Michael Ellerman a écrit :
> Hi Dan,
>
> Thanks for the patch.
>
> Dan Carpenter <dan.carpenter@oracle.com> writes:
>> The ipic_info[] array only has 95 elements so I have made the bounds
>> check smaller to prevent a read overflow. It was Smatch that found
>> this issue:
>>
>> arch/powerpc/sysdev/ipic.c:784 ipic_set_priority()
>> error: buffer overflow 'ipic_info' 95 <= 127
>>
>> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
>> ---
>> I wasn't able to find any callers of this code. Maybe we removed the
>> last one in commit b9f0f1bb2bca ("[POWERPC] Adapt ipic driver to new
>> host_ops interface, add set_irq_type to set IRQ sense"). So perhaps we
>> should just remove it. I'm not really comfortable doing that myself,
>> because I don't know the code well enough and can't build test
>> it properly.
>
> Hah wow, last usage removed in 2006!
>
> I don't see any mention of it since then, so I'll remove it. If it
> breaks something we can put it back.
>
> Can smatch help us find things like this that are defined non-static but
> never used?
>
I think we have to do that carrefully. Some of those functions might be
used by out-of-tree boards.
I'm thinking especially at ipic_get_mcp_status() and
ipic_set_mcp_status(). They are used in my 832x boards's machine check
handler to know when a machine check is a timeout from the 832x watchdog.
Christophe
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] powerpc/ipic: Fix a bounds check in ipic_set_priority()
2018-12-06 7:18 ` Christophe LEROY
@ 2018-12-06 8:12 ` Julia Lawall
2018-12-11 14:26 ` Dan Carpenter
2018-12-07 2:07 ` Michael Ellerman
1 sibling, 1 reply; 11+ messages in thread
From: Julia Lawall @ 2018-12-06 8:12 UTC (permalink / raw)
To: Christophe LEROY
Cc: Kim Phillips, kernel-janitors, Paul Mackerras, linuxppc-dev,
Dan Carpenter
[-- Attachment #1: Type: text/plain, Size: 2086 bytes --]
On Thu, 6 Dec 2018, Christophe LEROY wrote:
>
>
> Le 05/12/2018 à 04:26, Michael Ellerman a écrit :
> > Hi Dan,
> >
> > Thanks for the patch.
> >
> > Dan Carpenter <dan.carpenter@oracle.com> writes:
> > > The ipic_info[] array only has 95 elements so I have made the bounds
> > > check smaller to prevent a read overflow. It was Smatch that found
> > > this issue:
> > >
> > > arch/powerpc/sysdev/ipic.c:784 ipic_set_priority()
> > > error: buffer overflow 'ipic_info' 95 <= 127
> > >
> > > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> > > ---
> > > I wasn't able to find any callers of this code. Maybe we removed the
> > > last one in commit b9f0f1bb2bca ("[POWERPC] Adapt ipic driver to new
> > > host_ops interface, add set_irq_type to set IRQ sense"). So perhaps we
> > > should just remove it. I'm not really comfortable doing that myself,
> > > because I don't know the code well enough and can't build test
> > > it properly.
> >
> > Hah wow, last usage removed in 2006!
> >
> > I don't see any mention of it since then, so I'll remove it. If it
> > breaks something we can put it back.
> >
> > Can smatch help us find things like this that are defined non-static but
> > never used?
> >
>
> I think we have to do that carrefully. Some of those functions might be used
> by out-of-tree boards.
>
> I'm thinking especially at ipic_get_mcp_status() and ipic_set_mcp_status().
> They are used in my 832x boards's machine check handler to know when a machine
> check is a timeout from the 832x watchdog.
The message I have gotten in the past is that the Linux kernel doesn't
support code that is not used in the Linux kernel. However, if I were to
do this, I would send the code to the individual maintainers, who
presumably would know what is actually needed and what is not.
Perhaps a good sanity check would be if the code has been used in the
past. If there was a use in the past that has been removed, then perhaps
it is more likely that the function was intended for internal kernel use
rather than the case that you are describing.
julia
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] powerpc/ipic: Fix a bounds check in ipic_set_priority()
2018-12-05 3:26 ` Michael Ellerman
2018-12-05 8:11 ` Julia Lawall
2018-12-06 7:18 ` Christophe LEROY
@ 2018-12-06 9:34 ` Dan Carpenter
2 siblings, 0 replies; 11+ messages in thread
From: Dan Carpenter @ 2018-12-06 9:34 UTC (permalink / raw)
To: Michael Ellerman
Cc: Kim Phillips, kernel-janitors, Paul Mackerras, linuxppc-dev
On Wed, Dec 05, 2018 at 02:26:47PM +1100, Michael Ellerman wrote:
> Can smatch help us find things like this that are defined non-static but
> never used?
>
It's too tricky because it depends on the .config as well.
regards,
dan carpenter
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] powerpc/ipic: Fix a bounds check in ipic_set_priority()
2018-12-06 7:18 ` Christophe LEROY
2018-12-06 8:12 ` Julia Lawall
@ 2018-12-07 2:07 ` Michael Ellerman
2018-12-10 12:05 ` Christophe Leroy
1 sibling, 1 reply; 11+ messages in thread
From: Michael Ellerman @ 2018-12-07 2:07 UTC (permalink / raw)
To: Christophe LEROY, Dan Carpenter, Benjamin Herrenschmidt, Kim Phillips
Cc: linuxppc-dev, kernel-janitors, Paul Mackerras
Christophe LEROY <christophe.leroy@c-s.fr> writes:
> Le 05/12/2018 à 04:26, Michael Ellerman a écrit :
>> Hi Dan,
>>
>> Thanks for the patch.
>>
>> Dan Carpenter <dan.carpenter@oracle.com> writes:
>>> The ipic_info[] array only has 95 elements so I have made the bounds
>>> check smaller to prevent a read overflow. It was Smatch that found
>>> this issue:
>>>
>>> arch/powerpc/sysdev/ipic.c:784 ipic_set_priority()
>>> error: buffer overflow 'ipic_info' 95 <= 127
>>>
>>> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
>>> ---
>>> I wasn't able to find any callers of this code. Maybe we removed the
>>> last one in commit b9f0f1bb2bca ("[POWERPC] Adapt ipic driver to new
>>> host_ops interface, add set_irq_type to set IRQ sense"). So perhaps we
>>> should just remove it. I'm not really comfortable doing that myself,
>>> because I don't know the code well enough and can't build test
>>> it properly.
>>
>> Hah wow, last usage removed in 2006!
>>
>> I don't see any mention of it since then, so I'll remove it. If it
>> breaks something we can put it back.
>>
>> Can smatch help us find things like this that are defined non-static but
>> never used?
>>
>
> I think we have to do that carrefully. Some of those functions might be
> used by out-of-tree boards.
We don't keep unused code around for out-of-tree boards.
Either the out-of-tree code should be merged upstream, or you can
maintain whatever extra functions you need as part of your out-of-tree
code base.
> I'm thinking especially at ipic_get_mcp_status() and
> ipic_set_mcp_status(). They are used in my 832x boards's machine check
> handler to know when a machine check is a timeout from the 832x watchdog.
Thanks for pointing them out, I'll send a patch to remove them :)
But seriously, why is your machine check code not in-tree, is there some
reason you can't merge it?
cheers
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] powerpc/ipic: Fix a bounds check in ipic_set_priority()
2018-12-07 2:07 ` Michael Ellerman
@ 2018-12-10 12:05 ` Christophe Leroy
0 siblings, 0 replies; 11+ messages in thread
From: Christophe Leroy @ 2018-12-10 12:05 UTC (permalink / raw)
To: Michael Ellerman, Dan Carpenter, Benjamin Herrenschmidt, Kim Phillips
Cc: linuxppc-dev, kernel-janitors, Paul Mackerras
Le 07/12/2018 à 03:07, Michael Ellerman a écrit :
> Christophe LEROY <christophe.leroy@c-s.fr> writes:
>> Le 05/12/2018 à 04:26, Michael Ellerman a écrit :
>>> Hi Dan,
>>>
>>> Thanks for the patch.
>>>
>>> Dan Carpenter <dan.carpenter@oracle.com> writes:
>>>> The ipic_info[] array only has 95 elements so I have made the bounds
>>>> check smaller to prevent a read overflow. It was Smatch that found
>>>> this issue:
>>>>
>>>> arch/powerpc/sysdev/ipic.c:784 ipic_set_priority()
>>>> error: buffer overflow 'ipic_info' 95 <= 127
>>>>
>>>> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
>>>> ---
>>>> I wasn't able to find any callers of this code. Maybe we removed the
>>>> last one in commit b9f0f1bb2bca ("[POWERPC] Adapt ipic driver to new
>>>> host_ops interface, add set_irq_type to set IRQ sense"). So perhaps we
>>>> should just remove it. I'm not really comfortable doing that myself,
>>>> because I don't know the code well enough and can't build test
>>>> it properly.
>>>
>>> Hah wow, last usage removed in 2006!
>>>
>>> I don't see any mention of it since then, so I'll remove it. If it
>>> breaks something we can put it back.
>>>
>>> Can smatch help us find things like this that are defined non-static but
>>> never used?
>>>
>>
>> I think we have to do that carrefully. Some of those functions might be
>> used by out-of-tree boards.
>
> We don't keep unused code around for out-of-tree boards.
>
> Either the out-of-tree code should be merged upstream, or you can
> maintain whatever extra functions you need as part of your out-of-tree
> code base.
>
>> I'm thinking especially at ipic_get_mcp_status() and
>> ipic_set_mcp_status(). They are used in my 832x boards's machine check
>> handler to know when a machine check is a timeout from the 832x watchdog.
>
> Thanks for pointing them out, I'll send a patch to remove them :)
Lol :)
If you are doing the housework, you can remove
ipic_set_highest_priority() ipic_enable_mcp() and ipic_disable_mcp()
>
> But seriously, why is your machine check code not in-tree, is there some
> reason you can't merge it?
Maybe because you haven't merged it yet allthough I sent it more than
three minutes ago. :)
Seriously, that was just left over with other priorities, and also
because I'm not too happy about it because what I would really like is
that it kills the userland app if any but don't crash the system when it
is in interrupt or in idle.
But due to b96672dd840f ("powerpc: Machine check interrupt is a
non-maskable interrupt"), die_will_crash() doesn't work anymore. So for
the time being, the patch I sent is not killing anybody, just doing an
Oops for notification (note that die_will_crash() is used in the Opal
machine check handler, so it probably doesn't work anymore there either).
Christophe
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] powerpc/ipic: Fix a bounds check in ipic_set_priority()
2018-12-06 8:12 ` Julia Lawall
@ 2018-12-11 14:26 ` Dan Carpenter
0 siblings, 0 replies; 11+ messages in thread
From: Dan Carpenter @ 2018-12-11 14:26 UTC (permalink / raw)
To: Julia Lawall; +Cc: Kim Phillips, kernel-janitors, Paul Mackerras, linuxppc-dev
On Thu, Dec 06, 2018 at 09:12:12AM +0100, Julia Lawall wrote:
>
>
> On Thu, 6 Dec 2018, Christophe LEROY wrote:
>
> >
> >
> > Le 05/12/2018 à 04:26, Michael Ellerman a écrit :
> > > Hi Dan,
> > >
> > > Thanks for the patch.
> > >
> > > Dan Carpenter <dan.carpenter@oracle.com> writes:
> > > > The ipic_info[] array only has 95 elements so I have made the bounds
> > > > check smaller to prevent a read overflow. It was Smatch that found
> > > > this issue:
> > > >
> > > > arch/powerpc/sysdev/ipic.c:784 ipic_set_priority()
> > > > error: buffer overflow 'ipic_info' 95 <= 127
> > > >
> > > > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> > > > ---
> > > > I wasn't able to find any callers of this code. Maybe we removed the
> > > > last one in commit b9f0f1bb2bca ("[POWERPC] Adapt ipic driver to new
> > > > host_ops interface, add set_irq_type to set IRQ sense"). So perhaps we
> > > > should just remove it. I'm not really comfortable doing that myself,
> > > > because I don't know the code well enough and can't build test
> > > > it properly.
> > >
> > > Hah wow, last usage removed in 2006!
> > >
> > > I don't see any mention of it since then, so I'll remove it. If it
> > > breaks something we can put it back.
> > >
> > > Can smatch help us find things like this that are defined non-static but
> > > never used?
> > >
> >
> > I think we have to do that carrefully. Some of those functions might be used
> > by out-of-tree boards.
> >
> > I'm thinking especially at ipic_get_mcp_status() and ipic_set_mcp_status().
> > They are used in my 832x boards's machine check handler to know when a machine
> > check is a timeout from the 832x watchdog.
>
> The message I have gotten in the past is that the Linux kernel doesn't
> support code that is not used in the Linux kernel. However, if I were to
> do this, I would send the code to the individual maintainers, who
> presumably would know what is actually needed and what is not.
>
> Perhaps a good sanity check would be if the code has been used in the
> past. If there was a use in the past that has been removed, then perhaps
> it is more likely that the function was intended for internal kernel use
> rather than the case that you are describing.
Yeah. That's a good idea. I've been encouraging people who remove
"written to but not used" variables to say in the commit message
"variable foo has not been used since commit 123412341243 ("blah blah")."
It helps me review the patch as well.
regards,
dan carpenter
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2018-12-11 14:30 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-12-03 14:48 [PATCH] powerpc/ipic: Fix a bounds check in ipic_set_priority() Dan Carpenter
2018-12-05 3:26 ` Michael Ellerman
2018-12-05 8:11 ` Julia Lawall
2018-12-05 12:04 ` Michael Ellerman
2018-12-05 12:06 ` Julia Lawall
2018-12-06 7:18 ` Christophe LEROY
2018-12-06 8:12 ` Julia Lawall
2018-12-11 14:26 ` Dan Carpenter
2018-12-07 2:07 ` Michael Ellerman
2018-12-10 12:05 ` Christophe Leroy
2018-12-06 9:34 ` Dan Carpenter
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).