[PATCHSET] Add support for fall through attribute

classic Classic list List threaded Threaded
6 messages Options
Reply | Threaded
Open this post in threaded view
|

[PATCHSET] Add support for fall through attribute

Samba - samba-technical mailing list
Hello,

at least GCC offers a new warning: -Wimplicit-fallthrough

It also has an attribute to mark a case in a switch statement as fall through
to disable the warning!


Please review and push if OK.


Thanks!


Cheers,


        Andreas


--
Andreas Schneider                   GPG-ID: CC014E3D
Samba Team                             [hidden email]
www.samba.org

fall_through.patch1.txt (63K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCHSET] Add support for fall through attribute

Samba - samba-technical mailing list
On Thu, 2017-12-07 at 11:00 +0100, Andreas Schneider via samba-
technical wrote:
> Hello,
>
> at least GCC offers a new warning: -Wimplicit-fallthrough
>
> It also has an attribute to mark a case in a switch statement as fall through
> to disable the warning!
>
>
> Please review and push if OK.

This looks great.  I'll look over it more carefully tomorrow, but a big
thanks for investigating ways to make our C a little more safe!

Andrew Bartlett
--
Andrew Bartlett                       http://samba.org/~abartlet/
Authentication Developer, Samba Team  http://samba.org
Samba Developer, Catalyst IT          http://catalyst.net.nz/services/samba


Reply | Threaded
Open this post in threaded view
|

Re: [PATCHSET] Add support for fall through attribute

Samba - samba-technical mailing list
On Thursday, 7 December 2017 11:30:14 CET Andrew Bartlett via samba-technical
wrote:

> On Thu, 2017-12-07 at 11:00 +0100, Andreas Schneider via samba-
>
> technical wrote:
> > Hello,
> >
> > at least GCC offers a new warning: -Wimplicit-fallthrough
> >
> > It also has an attribute to mark a case in a switch statement as fall
> > through to disable the warning!
> >
> >
> > Please review and push if OK.
>
> This looks great.  I'll look over it more carefully tomorrow, but a big
> thanks for investigating ways to make our C a little more safe!
Ping :-)


Rebased patchset attached!


Thanks,


        Andreas

--
Andreas Schneider                   GPG-ID: CC014E3D
Samba Team                             [hidden email]
www.samba.org

fall_trough.patch2.txt (63K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCHSET] Add support for fall through attribute

Samba - samba-technical mailing list
On Tue, 2017-12-12 at 08:36 +0100, Andreas Schneider wrote:

> On Thursday, 7 December 2017 11:30:14 CET Andrew Bartlett via samba-technical
> wrote:
> > On Thu, 2017-12-07 at 11:00 +0100, Andreas Schneider via samba-
> >
> > technical wrote:
> > > Hello,
> > >
> > > at least GCC offers a new warning: -Wimplicit-fallthrough
> > >
> > > It also has an attribute to mark a case in a switch statement as fall
> > > through to disable the warning!
> > >
> > >
> > > Please review and push if OK.
> >
> > This looks great.  I'll look over it more carefully tomorrow, but a big
> > thanks for investigating ways to make our C a little more safe!
>
> Ping :-)
>
>
> Rebased patchset attached!

It looks like tomorrow will be patch review day.  I think I've got the
2012 schema patch set in a shape to be reviewed, so I can do some
review of other things.

Andrew Bartlett

--
Andrew Bartlett                       http://samba.org/~abartlet/
Authentication Developer, Samba Team  http://samba.org
Samba Developer, Catalyst IT          http://catalyst.net.nz/services/samba


Reply | Threaded
Open this post in threaded view
|

Re: [PATCHSET] Add support for fall through attribute

Samba - samba-technical mailing list
On Tue, 2017-12-12 at 22:11 +1300, Andrew Bartlett via samba-technical
wrote:

> On Tue, 2017-12-12 at 08:36 +0100, Andreas Schneider wrote:
> > On Thursday, 7 December 2017 11:30:14 CET Andrew Bartlett via samba-technical
> > wrote:
> > > On Thu, 2017-12-07 at 11:00 +0100, Andreas Schneider via samba-
> > >
> > > technical wrote:
> > > > Hello,
> > > >
> > > > at least GCC offers a new warning: -Wimplicit-fallthrough
> > > >
> > > > It also has an attribute to mark a case in a switch statement as fall
> > > > through to disable the warning!
> > > >
> > > >
> > > > Please review and push if OK.
> > >
> > > This looks great.  I'll look over it more carefully tomorrow, but a big
> > > thanks for investigating ways to make our C a little more safe!
> >
> > Ping :-)
> >
> >
> > Rebased patchset attached!
>
> It looks like tomorrow will be patch review day.  I think I've got the
> 2012 schema patch set in a shape to be reviewed, so I can do some
> review of other things.

I got about half-way though.  The challenge (as you know) is that just
making the warning quiet is really not what the review task is about,
it is checking that the FALL_THROUGH is actually correct and not a bug.

I'll have another go at it this week.

I'm particularly concerned about the tldap change because it implies
there isn't a testsuite for it.

Thanks for taking this on and helping make Samba a little more strict.

Andrew Bartlett

--
Andrew Bartlett                       http://samba.org/~abartlet/
Authentication Developer, Samba Team  http://samba.org
Samba Developer, Catalyst IT          http://catalyst.net.nz/services/samba


Reply | Threaded
Open this post in threaded view
|

Re: [PATCHSET] Add support for fall through attribute

Samba - samba-technical mailing list
On Wednesday, 13 December 2017 10:24:13 CET Andrew Bartlett wrote:

> On Tue, 2017-12-12 at 22:11 +1300, Andrew Bartlett via samba-technical
>
> wrote:
> > On Tue, 2017-12-12 at 08:36 +0100, Andreas Schneider wrote:
> > > On Thursday, 7 December 2017 11:30:14 CET Andrew Bartlett via
> > > samba-technical> >
> > > wrote:
> > > > On Thu, 2017-12-07 at 11:00 +0100, Andreas Schneider via samba-
> > > >
> > > > technical wrote:
> > > > > Hello,
> > > > >
> > > > > at least GCC offers a new warning: -Wimplicit-fallthrough
> > > > >
> > > > > It also has an attribute to mark a case in a switch statement as
> > > > > fall
> > > > > through to disable the warning!
> > > > >
> > > > >
> > > > > Please review and push if OK.
> > > >
> > > > This looks great.  I'll look over it more carefully tomorrow, but a
> > > > big
> > > > thanks for investigating ways to make our C a little more safe!
> > >
> > > Ping :-)
> > >
> > >
> > > Rebased patchset attached!
> >
> > It looks like tomorrow will be patch review day.  I think I've got the
> > 2012 schema patch set in a shape to be reviewed, so I can do some
> > review of other things.
>
> I got about half-way though.  The challenge (as you know) is that just
> making the warning quiet is really not what the review task is about,
> it is checking that the FALL_THROUGH is actually correct and not a bug.
>
> I'll have another go at it this week.

Thank you very much!

> I'm particularly concerned about the tldap change because it implies
> there isn't a testsuite for it.

source3/torture/torture.c see run_tldap()

/* test search filters against rootDSE */


There is a test but I don't see that this test is planned in selftest.


        Andreas

--
Andreas Schneider                   GPG-ID: CC014E3D
Samba Team                             [hidden email]
www.samba.org