On Thu, Mar 8, 2018 at 1:11 AM, Julia Lawall <julia.lawall@lip6.fr> wrote:

>
>
> On Wed, 7 Mar 2018, Arushi Singhal wrote:
>
> >
> >
> > On Wed, Mar 7, 2018 at 9:09 PM, David Miller <davem@davemloft.net>
> wrote:
> >       From: Arushi Singhal <arushisinghal19971997@gmail.com>
> >       Date: Sat, 3 Mar 2018 21:44:56 +0530
> >
> >       > Continue at the bottom of a loop are removed.
> >       > Issue found using drop_continue.cocci Coccinelle script.
> >       >
> >       > Signed-off-by: Arushi Singhal
> >       <arushisinghal19971997@gmail.com>
> >       > ---
> >       > Changes in v2:
> >       > - Braces is dropped from if with single statement.
> >
> >       Sorry there is no way I am applying this.
> >
> >       Just blindly removing continue statements that some static
> >       checker
> >       or compiler warning mentions is completely unacceptable.
> >
> >       Actually _LOOK_ at this code:
> >
> >       >               if(cards[i].vendor_id) {
> >       >                       for(j=0;j<3;j++)
> >       > -
> >        if(inb(ioaddr+cards[i].addr_offset+j) != cards[i].vendor_id[j])
> >       {
> >       > +
> >        if(inb(ioaddr+cards[i].addr_offset+j) != cards[i].vendor_id[j])
> >       >                                       release_region(ioaddr,
> >       cards[i].total_size);
> >       > -                                     continue;
> >       > -                       }
> >       >               }
> >
> >       The extraneous continue statement is the _least_ of the problems
> >       here.
> >
> >
> > Hello David
> >
> > Yes you are correct.
> >
> >
> >       This code, if the if() statement triggers, will release the
> >       ioaddr
> >       region and then _CONTINUE_ the for(j) loop, and try to access
> >       the
> >       registers using the same 'ioaddr' again.
> >
> >       That's actually a _REAL_ bug, and you're making it seem like
> >       the behavior is intentional by editing it like this.
> >
> >       And the bug probably exists because this entire sequence has
> >       misaligned closing curly braces.  It makes it look like
> >       the continue is for the outer loop, which would be fine.
> >
> >       But it's not.  It's for the inner loop, and this causes the "use
> >       ioaddr after releasing it" bug.
> >
> >
> > Yes I know it's for the inner loop.
> > But I am not able to find, where I am wrong here
>
> Arushi,
>
> You are preserving the current behavior and David is telling you that the
> current behavior is incorrect.  He doesn't want a patch that changes the
> code but preesrves the current (wrong) behavior, because that somehow
> contributes to the illusion that the incorrect code is correct.
>

Thanks Julia :) for explaining it further.
Arushi

>
> julia
>
> >
> > For example
> > BEFORE:-
> > for(i=1;i<10;i++)
> >        if (expr1)
> >           {
> >              statement;
> >              continue;
> >           }
> >
> > After My Patch
> > for(i=1;i<10;i++)
> >     if (expr1)
> >        statement;
> >
> > I am not understanding where I am getting wrong in understanding this.
> > For me both are equivalent.
> >
> > I Agree with Jakub reply:-
> > "This is an error handling path so the continue makes sense here to
> > indicate the processing can't ever fall through if more statements are
> > ever added to the loop.  But OK."
> >
> > Thanks
> > Arushi
> >
> >
> >       Please do not submit patches like this one, it makes for a lot
> >       of
> >       wasted auditing and review for people.  The onus is on you to
> >       read
> >       and understand the code you are editing before submitting your
> >       changes.
> >
> >       Thank you.
> >
> >
> > --
> > You received this message because you are subscribed to the Google Groups
> > "outreachy-kernel" group.
> > To unsubscribe from this group and stop receiving emails from it, send an
> > email to outreachy-kernel+unsubscribe@googlegroups.com.
> > To post to this group, send email to outreachy-kernel@googlegroups.com.
> > To view this discussion on the web visithttps://groups.google.
> com/d/msgid/outreachy-kernel/CA%2BXqjF8_axeTnahPmokxm
> > Dnm4NmWo8QWSEmLejN4fO1nAiDaBA%40mail.gmail.com.
> > For more options, visit https://groups.google.com/d/optout.
> >
> >

-- 
You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
To unsubscribe from this group and stop receiving emails from it, send an email to outreachy-kernel+unsubscribe@googlegroups.com.
To post to this group, send email to outreachy-kernel@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/CA%2BXqjF_9sMh2G6wX4SW75iXbUw%3DBt6J0AE4_rjqrNB_UAKcZtA%40mail.gmail.com.
For more options, visit https://groups.google.com/d/optout.