Making discarded-qualifiers a gcc error instead of warning?

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

Making discarded-qualifiers a gcc error instead of warning?

Will Fiveash-2
I'm in the process of making the Solaris compile of MIT krb more strict
and one thing I've noticed in the MIT build environment is that
-Wdiscarded-qualifiers is being treated by gcc as a warning, not an
error.  Given there are only three of these types of warnings in the
entire 1.14.3 code base I was wondering if y'all would consider making
discarded-qualifiers an error and adding a cast to the three lines in
the source files that need it?

Here are the actual warnings:

../../../../krb5-1.14.3/src/lib/krb5/krb/chpw.c: In function 'krb5int_mk_chpw_req':
../../../../krb5-1.14.3/src/lib/krb5/krb/chpw.c:32:18: warning: assignment discards 'const' qualifier from pointer target type [-Wdiscarded-qualifiers]
     clearpw.data = passwd;
                  ^
../../../../krb5-1.14.3/src/lib/krb5/krb/chpw.c: In function 'krb5int_mk_setpw_req':
../../../../krb5-1.14.3/src/lib/krb5/krb/chpw.c:305:23: warning: assignment discards 'const' qualifier from pointer target type [-Wdiscarded-qualifiers]
     req.password.data = passwd;
                       ^
../../../../../krb5-1.14.3/src/plugins/kdb/ldap/libkdb_ldap/kdb_ldap.c: In function 'has_rootdse_ava':
../../../../../krb5-1.14.3/src/plugins/kdb/ldap/libkdb_ldap/kdb_ldap.c:135:14: warning: assignment discards 'const' qualifier from pointer target type [-Wdiscarded-qualifiers]
     attrs[0] = attribute;
              ^
--
Will Fiveash
Oracle Solaris Software Engineer
_______________________________________________
krbdev mailing list             [hidden email]
https://mailman.mit.edu/mailman/listinfo/krbdev
Reply | Threaded
Open this post in threaded view
|

Re: Making discarded-qualifiers a gcc error instead of warning?

Will Fiveash-2
On Thu, Sep 01, 2016 at 05:56:15PM -0500, Will Fiveash wrote:
> I'm in the process of making the Solaris compile of MIT krb more strict
> and one thing I've noticed in the MIT build environment is that
> -Wdiscarded-qualifiers is being treated by gcc as a warning, not an
> error.  Given there are only three of these types of warnings in the
> entire 1.14.3 code base I was wondering if y'all would consider making
> discarded-qualifiers an error and adding a cast to the three lines in
> the source files that need it?

Related to this, the current MIT gcc settings allows:

#include <stdio.h>
#include <stdlib.h>

void
foo(char *arg)
{
        printf("arg is %p\n", arg);
        return;
}

int
main (int argc, char *argv[])
{
        char *bad;

        foo(&bad);  /* <----- this should just be bad */
}

to compile without a hard error (a -Wincompatible-pointer-types warning
is issued).  Shouldn't this bug also cause gcc to error out?

--
Will Fiveash
Oracle Solaris Software Engineer
_______________________________________________
krbdev mailing list             [hidden email]
https://mailman.mit.edu/mailman/listinfo/krbdev
Reply | Threaded
Open this post in threaded view
|

Re: Making discarded-qualifiers a gcc error instead of warning?

Benjamin Kaduk-2
On Thu, 1 Sep 2016, Will Fiveash wrote:

> On Thu, Sep 01, 2016 at 05:56:15PM -0500, Will Fiveash wrote:
> > I'm in the process of making the Solaris compile of MIT krb more strict
> > and one thing I've noticed in the MIT build environment is that
> > -Wdiscarded-qualifiers is being treated by gcc as a warning, not an
> > error.  Given there are only three of these types of warnings in the
> > entire 1.14.3 code base I was wondering if y'all would consider making
> > discarded-qualifiers an error and adding a cast to the three lines in
> > the source files that need it?
>
> Related to this, the current MIT gcc settings allows:
>
> #include <stdio.h>
> #include <stdlib.h>
>
> void
> foo(char *arg)
> {
> printf("arg is %p\n", arg);
> return;
> }
>
> int
> main (int argc, char *argv[])
> {
> char *bad;
>
> foo(&bad);  /* <----- this should just be bad */
> }
>
> to compile without a hard error (a -Wincompatible-pointer-types warning
> is issued).  Shouldn't this bug also cause gcc to error out?

char * is explicitly compatible with every (non-function) pointer type in
the C standard.  So this particular example should not necessarily error
out, though the analogous one with int* should.

-Ben
_______________________________________________
krbdev mailing list             [hidden email]
https://mailman.mit.edu/mailman/listinfo/krbdev
Reply | Threaded
Open this post in threaded view
|

Re: Making discarded-qualifiers a gcc error instead of warning?

Greg Hudson
In reply to this post by Will Fiveash-2
On 09/01/2016 08:23 PM, Will Fiveash wrote:
> On Thu, Sep 01, 2016 at 05:56:15PM -0500, Will Fiveash wrote:
>> I'm in the process of making the Solaris compile of MIT krb more strict
>> and one thing I've noticed in the MIT build environment is that
>> -Wdiscarded-qualifiers is being treated by gcc as a warning, not an
>> error.  Given there are only three of these types of warnings in the
>> entire 1.14.3 code base I was wondering if y'all would consider making
>> discarded-qualifiers an error and adding a cast to the three lines in
>> the source files that need it?

Sure.  Feel free to submit a pull request if you have time to do so;
otherwise I will try to get around to it.

> to compile without a hard error (a -Wincompatible-pointer-types warning
> is issued).  Shouldn't this bug also cause gcc to error out?

See:


https://github.com/krb5/krb5/commit/d69a3bd4c1d0c39a1f527c97f12bc53ea0cc1b8b

Ben wrote:
> char * is explicitly compatible with every (non-function) pointer type in
> the C standard.  So this particular example should not necessarily error
> out, though the analogous one with int* should.

I don't believe that's true.  You can cast any pointer to char * and
dereference it without violating strict aliasing (C99 section 6.5
paragraphs 6-7), but that doesn't mean char * is compatible with char **
(C99 section 6.7.5.1).
_______________________________________________
krbdev mailing list             [hidden email]
https://mailman.mit.edu/mailman/listinfo/krbdev
Reply | Threaded
Open this post in threaded view
|

Re: Making discarded-qualifiers a gcc error instead of warning?

Will Fiveash-2
On Fri, Sep 02, 2016 at 02:24:11AM -0400, Greg Hudson wrote:

> On 09/01/2016 08:23 PM, Will Fiveash wrote:
> > On Thu, Sep 01, 2016 at 05:56:15PM -0500, Will Fiveash wrote:
> >> I'm in the process of making the Solaris compile of MIT krb more strict
> >> and one thing I've noticed in the MIT build environment is that
> >> -Wdiscarded-qualifiers is being treated by gcc as a warning, not an
> >> error.  Given there are only three of these types of warnings in the
> >> entire 1.14.3 code base I was wondering if y'all would consider making
> >> discarded-qualifiers an error and adding a cast to the three lines in
> >> the source files that need it?
>
> Sure.  Feel free to submit a pull request if you have time to do so;
> otherwise I will try to get around to it.

I'll let you know, thanks.

> > to compile without a hard error (a -Wincompatible-pointer-types warning
> > is issued).  Shouldn't this bug also cause gcc to error out?
>
> See:
>
> https://github.com/krb5/krb5/commit/d69a3bd4c1d0c39a1f527c97f12bc53ea0cc1b8b

That sounds good to me.

> Ben wrote:
> > char * is explicitly compatible with every (non-function) pointer type in
> > the C standard.  So this particular example should not necessarily error
> > out, though the analogous one with int* should.
>
> I don't believe that's true.  You can cast any pointer to char * and
> dereference it without violating strict aliasing (C99 section 6.5
> paragraphs 6-7), but that doesn't mean char * is compatible with char **
> (C99 section 6.7.5.1).

That is my understanding as well.  If a function argument is void * on
the other hand then C does allow that function to be called with a
char/int/etc... ** argument (which is why I dislike use of void * as
argument types, it basically disables C static type checking).

--
Will Fiveash
Oracle Solaris Software Engineer
_______________________________________________
krbdev mailing list             [hidden email]
https://mailman.mit.edu/mailman/listinfo/krbdev
Reply | Threaded
Open this post in threaded view
|

Re: Making discarded-qualifiers a gcc error instead of warning?

Tom Yu
In reply to this post by Will Fiveash-2
Will Fiveash <[hidden email]> writes:

> ../../../../krb5-1.14.3/src/lib/krb5/krb/chpw.c: In function 'krb5int_mk_chpw_req':
> ../../../../krb5-1.14.3/src/lib/krb5/krb/chpw.c:32:18: warning: assignment discards 'const' qualifier from pointer target type [-Wdiscarded-qualifiers]
>      clearpw.data = passwd;
>                   ^
> ../../../../krb5-1.14.3/src/lib/krb5/krb/chpw.c: In function 'krb5int_mk_setpw_req':
> ../../../../krb5-1.14.3/src/lib/krb5/krb/chpw.c:305:23: warning: assignment discards 'const' qualifier from pointer target type [-Wdiscarded-qualifiers]
>      req.password.data = passwd;
>                        ^
> ../../../../../krb5-1.14.3/src/plugins/kdb/ldap/libkdb_ldap/kdb_ldap.c: In function 'has_rootdse_ava':
> ../../../../../krb5-1.14.3/src/plugins/kdb/ldap/libkdb_ldap/kdb_ldap.c:135:14: warning: assignment discards 'const' qualifier from pointer target type [-Wdiscarded-qualifiers]
>      attrs[0] = attribute;
>               ^

I think e534f688db29945e4efde425f62ce7dae4608f6f (on master) already
fixed the chpw.c cases, and 66acc3b529f997f902bf11d3004ddc065d5bea9f
(also on master) fixed the kdb_ldap.c case.  They're not currently
marked for pullup to release branches, but I'm willing to consider doing
so if they're not too difficult to back port.

-Tom
_______________________________________________
krbdev mailing list             [hidden email]
https://mailman.mit.edu/mailman/listinfo/krbdev
Reply | Threaded
Open this post in threaded view
|

Re: Making discarded-qualifiers a gcc error instead of warning?

Will Fiveash-2
On Fri, Sep 02, 2016 at 04:03:46PM -0400, Tom Yu wrote:

> Will Fiveash <[hidden email]> writes:
>
> > ../../../../krb5-1.14.3/src/lib/krb5/krb/chpw.c: In function 'krb5int_mk_chpw_req':
> > ../../../../krb5-1.14.3/src/lib/krb5/krb/chpw.c:32:18: warning: assignment discards 'const' qualifier from pointer target type [-Wdiscarded-qualifiers]
> >      clearpw.data = passwd;
> >                   ^
> > ../../../../krb5-1.14.3/src/lib/krb5/krb/chpw.c: In function 'krb5int_mk_setpw_req':
> > ../../../../krb5-1.14.3/src/lib/krb5/krb/chpw.c:305:23: warning: assignment discards 'const' qualifier from pointer target type [-Wdiscarded-qualifiers]
> >      req.password.data = passwd;
> >                        ^
> > ../../../../../krb5-1.14.3/src/plugins/kdb/ldap/libkdb_ldap/kdb_ldap.c: In function 'has_rootdse_ava':
> > ../../../../../krb5-1.14.3/src/plugins/kdb/ldap/libkdb_ldap/kdb_ldap.c:135:14: warning: assignment discards 'const' qualifier from pointer target type [-Wdiscarded-qualifiers]
> >      attrs[0] = attribute;
> >               ^
>
> I think e534f688db29945e4efde425f62ce7dae4608f6f (on master) already
> fixed the chpw.c cases, and 66acc3b529f997f902bf11d3004ddc065d5bea9f
> (also on master) fixed the kdb_ldap.c case.  They're not currently
> marked for pullup to release branches, but I'm willing to consider doing
> so if they're not too difficult to back port.

Thanks for looking at that.  I don't think you need to worry about
backporting those cast fixes for us, we can just patch it if we need to
(still mulling over how/when we will go about making our build env.
stricter).  I am glad to see that y'all have been working towards
tightening up the build environment as that should make building MIT
krb5 code easier next time we update to newer MIT krb5 source.

--
Will Fiveash
Oracle Solaris Software Engineer
_______________________________________________
krbdev mailing list             [hidden email]
https://mailman.mit.edu/mailman/listinfo/krbdev