diff mbox

parisc: Remove FIXME comment

Message ID 1405715823-17492-1-git-send-email-xerofoify@gmail.com (mailing list archive)
State Rejected
Headers show

Commit Message

Nick July 18, 2014, 8:37 p.m. UTC
The comment for size of frame not being needed is incorrect , the
function called needs this parameter.

Signed-off-by: Nicholas Krause <xerofoify@gmail.com>
---
 arch/parisc/kernel/signal.c | 1 -
 1 file changed, 1 deletion(-)

Comments

Helge Deller July 18, 2014, 8:49 p.m. UTC | #1
On 07/18/2014 10:37 PM, Nicholas Krause wrote:
> The comment for size of frame not being needed is incorrect , the
> function called needs this parameter.

Thanks for the patch Nicholas.

It has been queued up:
https://patchwork.kernel.org/patch/4587631/
and
https://patchwork.kernel.org/patch/4486231/

I will apply it at some point, but not yet.
There are other (more important) patches upcoming regarding the 
signal handling,  and I don't want to break those at the moment.

Helge

> 
> Signed-off-by: Nicholas Krause <xerofoify@gmail.com>
> ---
>  arch/parisc/kernel/signal.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/arch/parisc/kernel/signal.c b/arch/parisc/kernel/signal.c
> index 1cba8f2..1d550b5 100644
> --- a/arch/parisc/kernel/signal.c
> +++ b/arch/parisc/kernel/signal.c
> @@ -240,7 +240,6 @@ setup_rt_frame(int sig, struct k_sigaction *ka, siginfo_t *info,
>  #endif
>  	
>  	usp = (regs->gr[30] & ~(0x01UL));
> -	/*FIXME: frame_size parameter is unused, remove it. */
>  	frame = get_sigframe(ka, usp, sizeof(*frame));
>  
>  	DBG(1,"SETUP_RT_FRAME: START\n");
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-parisc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
James Bottomley July 18, 2014, 9:03 p.m. UTC | #2
On Fri, 2014-07-18 at 16:37 -0400, Nicholas Krause wrote:
> The comment for size of frame not being needed is incorrect , the
> function called needs this parameter.

Actually, that's not correct.  The point of the FIXME is that fram_size
is only used in a debug print and could be eliminated since the
internals of the function excluding the debugging statements don't use
it.

James


--
To unsubscribe from this list: send the line "unsubscribe linux-parisc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Nick July 19, 2014, 2:25 a.m. UTC | #3
On Fri, Jul 18, 2014 at 5:03 PM, James Bottomley
<James.Bottomley@hansenpartnership.com> wrote:
> On Fri, 2014-07-18 at 16:37 -0400, Nicholas Krause wrote:
>> The comment for size of frame not being needed is incorrect , the
>> function called needs this parameter.
>
> Actually, that's not correct.  The point of the FIXME is that fram_size
> is only used in a debug print and could be eliminated since the
> internals of the function excluding the debugging statements don't use
> it.
>
> James
>
>

So I need to need a patch removing the parameter from the  function
and this part of the code?
Cheers Nick
--
To unsubscribe from this list: send the line "unsubscribe linux-parisc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
James Bottomley July 19, 2014, 5:20 a.m. UTC | #4
On Fri, 2014-07-18 at 22:25 -0400, Nick Krause wrote:
> On Fri, Jul 18, 2014 at 5:03 PM, James Bottomley
> <James.Bottomley@hansenpartnership.com> wrote:
> > On Fri, 2014-07-18 at 16:37 -0400, Nicholas Krause wrote:
> >> The comment for size of frame not being needed is incorrect , the
> >> function called needs this parameter.
> >
> > Actually, that's not correct.  The point of the FIXME is that fram_size
> > is only used in a debug print and could be eliminated since the
> > internals of the function excluding the debugging statements don't use
> > it.
> >
> > James
> >
> >
> 
> So I need to need a patch removing the parameter from the  function
> and this part of the code?

Well, no, I'm not sure there's any action to take.  The FIXME reminds us
that there's no actual use of the frame size in the function body except
for the debugging prints.  The parameter could be removed if everyone
who debugs the kernel agrees either to remove the debug code or remove
the frame pointer from the prints, but it's probably not worth bothering
about, which is why it's still there.

James


--
To unsubscribe from this list: send the line "unsubscribe linux-parisc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Nick July 19, 2014, 5:28 a.m. UTC | #5
On Sat, Jul 19, 2014 at 1:20 AM, James Bottomley
<James.Bottomley@hansenpartnership.com> wrote:
> On Fri, 2014-07-18 at 22:25 -0400, Nick Krause wrote:
>> On Fri, Jul 18, 2014 at 5:03 PM, James Bottomley
>> <James.Bottomley@hansenpartnership.com> wrote:
>> > On Fri, 2014-07-18 at 16:37 -0400, Nicholas Krause wrote:
>> >> The comment for size of frame not being needed is incorrect , the
>> >> function called needs this parameter.
>> >
>> > Actually, that's not correct.  The point of the FIXME is that fram_size
>> > is only used in a debug print and could be eliminated since the
>> > internals of the function excluding the debugging statements don't use
>> > it.
>> >
>> > James
>> >
>> >
>>
>> So I need to need a patch removing the parameter from the  function
>> and this part of the code?
>
> Well, no, I'm not sure there's any action to take.  The FIXME reminds us
> that there's no actual use of the frame size in the function body except
> for the debugging prints.  The parameter could be removed if everyone
> who debugs the kernel agrees either to remove the debug code or remove
> the frame pointer from the prints, but it's probably not worth bothering
> about, which is why it's still there.
>
> James
>
>
Ok then , seems its going through I can however write some comments to
explain this
if needed.
Cheers Nick
--
To unsubscribe from this list: send the line "unsubscribe linux-parisc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/arch/parisc/kernel/signal.c b/arch/parisc/kernel/signal.c
index 1cba8f2..1d550b5 100644
--- a/arch/parisc/kernel/signal.c
+++ b/arch/parisc/kernel/signal.c
@@ -240,7 +240,6 @@  setup_rt_frame(int sig, struct k_sigaction *ka, siginfo_t *info,
 #endif
 	
 	usp = (regs->gr[30] & ~(0x01UL));
-	/*FIXME: frame_size parameter is unused, remove it. */
 	frame = get_sigframe(ka, usp, sizeof(*frame));
 
 	DBG(1,"SETUP_RT_FRAME: START\n");