Making discarded-qualifiers a gcc error instead of warning?

[Will Fiveash]

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]

Post by Will Fiveash
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?

[Benjamin Kaduk]

Post by Will Fiveash

Post by Will Fiveash
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?

#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 ***@mit.edu
https://mailman.mit.edu/mailman/listinfo/krbdev

[Greg Hudson]

Post by Will Fiveash

Post by Will Fiveash
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.

Post by Will Fiveash
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

Post by Will Fiveash
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 ***@mit.edu
https://mailman.mit.edu/mailman/listinfo/krbdev

[Will Fiveash]

Post by Greg Hudson

Post by Will Fiveash

Post by Will Fiveash
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.

Post by Greg Hudson

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

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

That sounds good to me.

Post by Greg Hudson

Post by Will Fiveash
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).

[Tom Yu]

Post by Will Fiveash
../../../../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: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: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 ***@mit.edu
https://mailman.mit.edu/mailman/listinfo/krbdev

[Will Fiveash]

Post by Tom Yu

Post by Will Fiveash
../../../../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: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: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.