[PATCH]: Fix problematic -Wcast-qual cases using new CONST_CAST macro
Kaveh R. GHAZI
ghazi@caip.rutgers.edu
Fri Jul 27 17:24:00 GMT 2007
On Fri, 27 Jul 2007, Richard Guenther wrote:
> On 7/27/07, Kaveh R. GHAZI <ghazi@caip.rutgers.edu> wrote:
> > +union gcc_constcast
> > +{
> > + const void *cv;
> > + void *v;
> > +};
> > +#define CONST_CAST(X) ((__extension__(union gcc_constcast)(const void *)(X)).v)
>
> Can you explain in how this is better than using an explicit cast to the
> non-const version like in memcpy ((char *)x, ...)?
Sure. Besides my main point which is silencing -Wcast-qual warnings, the
CONST_CAST macro is grep-able. I.e. you can search for the few places
where it's used and audit if they are correctly applied. You cannot do
that with regular C-style casts. Another reason is that with C-style
casts there isn't any indication of why it's there. The CONST_CAST macro
comments itself to someone reading the code. (These are some of the
rationales for the creation of C++'s cast operators.) The macro isn't
type-safe like the C++ operator is, but it's a poor man's C option. :-)
I agree a new __typeof (__main_typeof?) that only strips the qualifiers
would be a good refinement for type-safety. But that's something which
can be altered in the macro definition as a followup if/when someone
implements it.
> At least it cannot detect wrongly typed arguments anymore due to the use
> of void*. ATM we could do the following:
>
> #define CONST_CAST(X) ((__extension__((union { __typeof__(X) _q; void
> *v; })(X)).v)
This version uses the macro definition you suggested in system.h. The
rest of the patch is unchanged. Okay for mainline?
Thanks,
--Kaveh
2007-07-24 Kaveh R. Ghazi <ghazi@caip.rutgers.edu>
* system.h (CONST_CAST): New.
* c-decl.c (c_make_fname_decl): Use it.
* c-lex.c (cb_ident, lex_string): Likewise.
* c-typeck.c (free_all_tagged_tu_seen_up_to): Likewise.
* gcc.c (set_spec, read_specs, for_each_path, execute, do_spec_1,
give_switch, set_multilib_dir): Likewise.
* gengtype-parse.c (string_seq, typedef_name): Likewise.
* passes.c (execute_one_pass): Likewise.
* prefix.c (update_path): Likewise.
* pretty-print.c (pp_base_destroy_prefix): Likewise.
* tree.c (build_string): Likewise.
cp:
* call.c (name_as_c_string): Use CONST_CAST.
* decl.c (build_decl): Likewise.
* parser.c (cp_parser_string_literal): Likewise.
fortran:
* gfortranspec.c (lang_specific_driver): Use CONST_CAST.
* options.c (gfc_post_options): Likewise.
* parse.c (parse_omp_structured_block): Likewise.
* st.c (gfc_free_statement): Likewise.
java:
* jcf-parse.c (read_class, java_parse_file): Use CONST_CAST.
* jcf.h (JCF_FINISH): Likewise.
diff -rup orig/egcc-SVN20070727/gcc/system.h egcc-SVN20070727/gcc/system.h
--- orig/egcc-SVN20070727/gcc/system.h 2007-07-26 23:04:08.000000000 -0400
+++ egcc-SVN20070727/gcc/system.h 2007-07-27 12:32:41.799916226 -0400
@@ -766,4 +766,18 @@ extern void fancy_abort (const char *, i
#endif /* GCC >= 3.0 */
+/* This macro allows casting away const-ness to pass -Wcast-qual
+ warnings. DO NOT USE THIS UNLESS YOU REALLY HAVE TO! It should
+ only be used in certain specific cases. One valid case is where
+ the C standard definitions or prototypes force you to. E.g. if you
+ need to free a const object, or if you pass a const string to
+ execv, et al. Another valid use would be in an allocation function
+ that creates const objects that need to be initialized. Most other
+ cases should be viewed with extreme caution. */
+#ifdef __GNUC__
+#define CONST_CAST(X) ((__extension__(union {__typeof(X)_q; void *_v;})(X))._v)
+#else
+#define CONST_CAST(X) ((void*)(X))
+#endif
+
#endif /* ! GCC_SYSTEM_H */
diff -rup orig/egcc-SVN20070727/gcc/c-decl.c egcc-SVN20070727/gcc/c-decl.c
--- orig/egcc-SVN20070727/gcc/c-decl.c 2007-07-26 23:03:51.000000000 -0400
+++ egcc-SVN20070727/gcc/c-decl.c 2007-07-27 12:22:09.221318916 -0400
@@ -2811,7 +2811,7 @@ c_make_fname_decl (tree id, int type_dep
DECL_ARTIFICIAL (decl) = 1;
init = build_string (length + 1, name);
- free ((char *) name);
+ free (CONST_CAST (name));
TREE_TYPE (init) = type;
DECL_INITIAL (decl) = init;
diff -rup orig/egcc-SVN20070727/gcc/c-lex.c egcc-SVN20070727/gcc/c-lex.c
--- orig/egcc-SVN20070727/gcc/c-lex.c 2007-07-26 23:03:30.000000000 -0400
+++ egcc-SVN20070727/gcc/c-lex.c 2007-07-27 12:22:09.224377056 -0400
@@ -186,7 +186,7 @@ cb_ident (cpp_reader * ARG_UNUSED (pfile
if (cpp_interpret_string (pfile, str, 1, &cstr, false))
{
ASM_OUTPUT_IDENT (asm_out_file, (const char *) cstr.text);
- free ((void *) cstr.text);
+ free (CONST_CAST (cstr.text));
}
}
#endif
@@ -811,7 +811,7 @@ lex_string (const cpp_token *tok, tree *
(parse_in, strs, concats + 1, &istr, wide))
{
value = build_string (istr.len, (const char *) istr.text);
- free ((void *) istr.text);
+ free (CONST_CAST (istr.text));
if (c_lex_string_translate == -1)
{
@@ -832,7 +832,7 @@ lex_string (const cpp_token *tok, tree *
*valp = build_string (istr.len, (const char *) istr.text);
valp = &TREE_CHAIN (*valp);
}
- free ((void *) istr.text);
+ free (CONST_CAST (istr.text));
}
}
else
diff -rup orig/egcc-SVN20070727/gcc/c-typeck.c egcc-SVN20070727/gcc/c-typeck.c
--- orig/egcc-SVN20070727/gcc/c-typeck.c 2007-07-26 23:03:57.000000000 -0400
+++ egcc-SVN20070727/gcc/c-typeck.c 2007-07-27 12:22:09.235239783 -0400
@@ -1026,7 +1026,7 @@ free_all_tagged_tu_seen_up_to (const str
const struct tagged_tu_seen_cache *const tu1
= (const struct tagged_tu_seen_cache *) tu;
tu = tu1->next;
- free ((void *)tu1);
+ free (CONST_CAST (tu1));
}
tagged_tu_seen_base = tu_til;
}
diff -rup orig/egcc-SVN20070727/gcc/cp/call.c egcc-SVN20070727/gcc/cp/call.c
--- orig/egcc-SVN20070727/gcc/cp/call.c 2007-07-05 23:03:38.000000000 -0400
+++ egcc-SVN20070727/gcc/cp/call.c 2007-07-27 12:22:09.243329235 -0400
@@ -5367,7 +5367,7 @@ name_as_c_string (tree name, tree type,
if (IDENTIFIER_CTOR_OR_DTOR_P (name))
{
pretty_name
- = (char *) IDENTIFIER_POINTER (constructor_name (type));
+ = (char *) CONST_CAST (IDENTIFIER_POINTER (constructor_name (type)));
/* For a destructor, add the '~'. */
if (name == complete_dtor_identifier
|| name == base_dtor_identifier
@@ -5388,7 +5388,7 @@ name_as_c_string (tree name, tree type,
*free_p = true;
}
else
- pretty_name = (char *) IDENTIFIER_POINTER (name);
+ pretty_name = (char *) CONST_CAST (IDENTIFIER_POINTER (name));
return pretty_name;
}
diff -rup orig/egcc-SVN20070727/gcc/cp/decl.c egcc-SVN20070727/gcc/cp/decl.c
--- orig/egcc-SVN20070727/gcc/cp/decl.c 2007-07-25 15:13:27.000000000 -0400
+++ egcc-SVN20070727/gcc/cp/decl.c 2007-07-27 12:22:09.259396354 -0400
@@ -3369,7 +3369,7 @@ cp_make_fname_decl (tree id, int type_de
tree decl = build_decl (VAR_DECL, id, type);
if (name)
- free ((char *) name);
+ free (CONST_CAST (name));
/* As we're using pushdecl_with_scope, we must set the context. */
DECL_CONTEXT (decl) = current_function_decl;
diff -rup orig/egcc-SVN20070727/gcc/cp/parser.c egcc-SVN20070727/gcc/cp/parser.c
--- orig/egcc-SVN20070727/gcc/cp/parser.c 2007-07-25 15:52:37.000000000 -0400
+++ egcc-SVN20070727/gcc/cp/parser.c 2007-07-27 12:22:09.285787196 -0400
@@ -2919,8 +2919,8 @@ cp_parser_string_literal (cp_parser *par
if ((translate ? cpp_interpret_string : cpp_interpret_string_notranslate)
(parse_in, strs, count, &istr, wide))
{
- value = build_string (istr.len, (char *)istr.text);
- free ((void *)istr.text);
+ value = build_string (istr.len, (const char *)istr.text);
+ free (CONST_CAST (istr.text));
TREE_TYPE (value) = wide ? wchar_array_type_node : char_array_type_node;
value = fix_string_type (value);
diff -rup orig/egcc-SVN20070727/gcc/fortran/gfortranspec.c egcc-SVN20070727/gcc/fortran/gfortranspec.c
--- orig/egcc-SVN20070727/gcc/fortran/gfortranspec.c 2007-06-27 19:45:34.000000000 -0400
+++ egcc-SVN20070727/gcc/fortran/gfortranspec.c 2007-07-27 12:22:09.288230636 -0400
@@ -303,7 +303,7 @@ lang_specific_driver (int *in_argc, cons
g77_xargc = argc;
g77_xargv = argv;
g77_newargc = 0;
- g77_newargv = (const char **) argv;
+ g77_newargv = (const char **) CONST_CAST (argv);
/* First pass through arglist.
diff -rup orig/egcc-SVN20070727/gcc/fortran/options.c egcc-SVN20070727/gcc/fortran/options.c
--- orig/egcc-SVN20070727/gcc/fortran/options.c 2007-07-15 23:02:34.000000000 -0400
+++ egcc-SVN20070727/gcc/fortran/options.c 2007-07-27 12:22:09.289546057 -0400
@@ -240,7 +240,7 @@ gfc_post_options (const char **pfilename
gfc_add_include_path (".", true);
if (canon_source_file != gfc_source_file)
- gfc_free ((void *) canon_source_file);
+ gfc_free (CONST_CAST (canon_source_file));
/* Decide which form the file will be read in as. */
diff -rup orig/egcc-SVN20070727/gcc/fortran/parse.c egcc-SVN20070727/gcc/fortran/parse.c
--- orig/egcc-SVN20070727/gcc/fortran/parse.c 2007-07-22 23:02:56.000000000 -0400
+++ egcc-SVN20070727/gcc/fortran/parse.c 2007-07-27 12:22:09.293116555 -0400
@@ -2609,7 +2609,7 @@ parse_omp_structured_block (gfc_statemen
&& strcmp (cp->ext.omp_name, new_st.ext.omp_name) != 0))
gfc_error ("Name after !$omp critical and !$omp end critical does "
"not match at %C");
- gfc_free ((char *) new_st.ext.omp_name);
+ gfc_free (CONST_CAST (new_st.ext.omp_name));
break;
case EXEC_OMP_END_SINGLE:
cp->ext.omp_clauses->lists[OMP_LIST_COPYPRIVATE]
diff -rup orig/egcc-SVN20070727/gcc/fortran/st.c egcc-SVN20070727/gcc/fortran/st.c
--- orig/egcc-SVN20070727/gcc/fortran/st.c 2007-01-20 20:01:25.000000000 -0500
+++ egcc-SVN20070727/gcc/fortran/st.c 2007-07-27 12:22:09.294050288 -0400
@@ -174,7 +174,7 @@ gfc_free_statement (gfc_code *p)
break;
case EXEC_OMP_CRITICAL:
- gfc_free ((char *) p->ext.omp_name);
+ gfc_free (CONST_CAST (p->ext.omp_name));
break;
case EXEC_OMP_FLUSH:
diff -rup orig/egcc-SVN20070727/gcc/gcc.c egcc-SVN20070727/gcc/gcc.c
--- orig/egcc-SVN20070727/gcc/gcc.c 2007-07-26 23:03:38.000000000 -0400
+++ egcc-SVN20070727/gcc/gcc.c 2007-07-27 12:22:09.303780116 -0400
@@ -1876,7 +1876,7 @@ set_spec (const char *name, const char *
/* Free the old spec. */
if (old_spec && sl->alloc_p)
- free ((void *) old_spec);
+ free (CONST_CAST(old_spec));
sl->alloc_p = 1;
}
@@ -2181,7 +2181,7 @@ read_specs (const char *filename, int ma
set_spec (p2, *(sl->ptr_spec));
if (sl->alloc_p)
- free ((void *) *(sl->ptr_spec));
+ free (CONST_CAST (*(sl->ptr_spec)));
*(sl->ptr_spec) = "";
sl->alloc_p = 0;
@@ -2531,18 +2531,18 @@ for_each_path (const struct path_prefix
Don't repeat any we have already seen. */
if (multi_dir)
{
- free ((char *) multi_dir);
+ free (CONST_CAST (multi_dir));
multi_dir = NULL;
- free ((char *) multi_suffix);
+ free (CONST_CAST (multi_suffix));
multi_suffix = machine_suffix;
- free ((char *) just_multi_suffix);
+ free (CONST_CAST (just_multi_suffix));
just_multi_suffix = just_machine_suffix;
}
else
skip_multi_dir = true;
if (multi_os_dir)
{
- free ((char *) multi_os_dir);
+ free (CONST_CAST (multi_os_dir));
multi_os_dir = NULL;
}
else
@@ -2551,12 +2551,12 @@ for_each_path (const struct path_prefix
if (multi_dir)
{
- free ((char *) multi_dir);
- free ((char *) multi_suffix);
- free ((char *) just_multi_suffix);
+ free (CONST_CAST (multi_dir));
+ free (CONST_CAST (multi_suffix));
+ free (CONST_CAST (just_multi_suffix));
}
if (multi_os_dir)
- free ((char *) multi_os_dir);
+ free (CONST_CAST (multi_os_dir));
if (ret != path)
free (path);
return ret;
@@ -2963,7 +2963,7 @@ execute (void)
errmsg = pex_run (pex,
((i + 1 == n_commands ? PEX_LAST : 0)
| (string == commands[i].prog ? PEX_SEARCH : 0)),
- string, (char * const *) commands[i].argv,
+ string, (char * const *) CONST_CAST (commands[i].argv),
NULL, NULL, &err);
if (errmsg != NULL)
{
@@ -2977,7 +2977,7 @@ execute (void)
}
if (string != commands[i].prog)
- free ((void *) string);
+ free (CONST_CAST (string));
}
execution_count++;
@@ -5041,7 +5041,7 @@ do_spec_1 (const char *spec, int inswitc
for (i = 0, j = 0; i < max; i++)
if (outfiles[i])
{
- argv[j] = (char *) outfiles[i];
+ argv[j] = (char *) CONST_CAST (outfiles[i]);
j++;
}
argv[j] = NULL;
@@ -6017,13 +6017,13 @@ give_switch (int switchnum, int omit_fir
while (length-- && !IS_DIR_SEPARATOR (arg[length]))
if (arg[length] == '.')
{
- ((char *)arg)[length] = 0;
+ ((char *)CONST_CAST(arg))[length] = 0;
dot = 1;
break;
}
do_spec_1 (arg, 1, NULL);
if (dot)
- ((char *)arg)[length] = '.';
+ ((char *)CONST_CAST(arg))[length] = '.';
do_spec_1 (suffix_subst, 1, NULL);
}
else
@@ -7476,7 +7476,7 @@ set_multilib_dir (void)
if (multilib_dir == NULL && multilib_os_dir != NULL
&& strcmp (multilib_os_dir, ".") == 0)
{
- free ((char *) multilib_os_dir);
+ free (CONST_CAST (multilib_os_dir));
multilib_os_dir = NULL;
}
else if (multilib_dir != NULL && multilib_os_dir == NULL)
diff -rup orig/egcc-SVN20070727/gcc/gengtype-parse.c egcc-SVN20070727/gcc/gengtype-parse.c
--- orig/egcc-SVN20070727/gcc/gengtype-parse.c 2007-07-26 23:03:55.000000000 -0400
+++ egcc-SVN20070727/gcc/gengtype-parse.c 2007-07-27 12:22:09.306731752 -0400
@@ -197,9 +197,9 @@ string_seq (void)
l1 = strlen (s1);
l2 = strlen (s2);
- buf = XRESIZEVEC (char, s1, l1 + l2 + 1);
+ buf = XRESIZEVEC (char, CONST_CAST(s1), l1 + l2 + 1);
memcpy (buf + l1, s2, l2 + 1);
- XDELETE (s2);
+ XDELETE (CONST_CAST (s2));
s1 = buf;
}
return s1;
@@ -221,8 +221,8 @@ typedef_name (void)
c2 = require (ID);
require (')');
r = concat ("VEC_", c1, "_", c2, (char *)0);
- free ((void *)c1);
- free ((void *)c2);
+ free (CONST_CAST (c1));
+ free (CONST_CAST (c2));
return r;
}
else
diff -rup orig/egcc-SVN20070727/gcc/java/jcf-parse.c egcc-SVN20070727/gcc/java/jcf-parse.c
--- orig/egcc-SVN20070727/gcc/java/jcf-parse.c 2007-07-25 15:13:16.000000000 -0400
+++ egcc-SVN20070727/gcc/java/jcf-parse.c 2007-07-27 12:22:09.310355301 -0400
@@ -1302,7 +1302,7 @@ read_class (tree name)
if (path_name == 0)
return 0;
else
- free((char *) path_name);
+ free(CONST_CAST (path_name));
}
current_jcf = jcf;
@@ -1784,7 +1784,7 @@ java_parse_file (int set_yydebug ATTRIBU
file_list = list;
}
else
- list = (char *) main_input_filename;
+ list = (char *) CONST_CAST (main_input_filename);
while (list)
{
diff -rup orig/egcc-SVN20070727/gcc/java/jcf.h egcc-SVN20070727/gcc/java/jcf.h
--- orig/egcc-SVN20070727/gcc/java/jcf.h 2007-01-13 20:02:03.000000000 -0500
+++ egcc-SVN20070727/gcc/java/jcf.h 2007-07-27 12:22:09.311437309 -0400
@@ -165,8 +165,8 @@ typedef struct JCF GTY(()) {
#define JCF_FINISH(JCF) { \
CPOOL_FINISH(&(JCF)->cpool); \
if ((JCF)->buffer) free ((JCF)->buffer); \
- if ((JCF)->filename) free ((char *) (JCF)->filename); \
- if ((JCF)->classname) free ((char *) (JCF)->classname); \
+ if ((JCF)->filename) free (CONST_CAST ((JCF)->filename)); \
+ if ((JCF)->classname) free (CONST_CAST ((JCF)->classname)); \
(JCF)->finished = 1; }
#define CPOOL_INIT(CPOOL) \
diff -rup orig/egcc-SVN20070727/gcc/passes.c egcc-SVN20070727/gcc/passes.c
--- orig/egcc-SVN20070727/gcc/passes.c 2007-07-26 23:04:10.000000000 -0400
+++ egcc-SVN20070727/gcc/passes.c 2007-07-27 12:22:09.313279731 -0400
@@ -1152,7 +1152,7 @@ execute_one_pass (struct tree_opt_pass *
/* Flush and close dump file. */
if (dump_file_name)
{
- free ((char *) dump_file_name);
+ free (CONST_CAST (dump_file_name));
dump_file_name = NULL;
}
diff -rup orig/egcc-SVN20070727/gcc/prefix.c egcc-SVN20070727/gcc/prefix.c
--- orig/egcc-SVN20070727/gcc/prefix.c 2007-07-26 23:04:13.000000000 -0400
+++ egcc-SVN20070727/gcc/prefix.c 2007-07-27 12:22:09.314993723 -0400
@@ -266,7 +266,7 @@ update_path (const char *path, const cha
result = concat (key, &path[len], NULL);
if (free_key)
- free ((char *) key);
+ free (CONST_CAST (key));
result = translate_name (result);
}
else
diff -rup orig/egcc-SVN20070727/gcc/pretty-print.c egcc-SVN20070727/gcc/pretty-print.c
--- orig/egcc-SVN20070727/gcc/pretty-print.c 2007-07-26 23:03:41.000000000 -0400
+++ egcc-SVN20070727/gcc/pretty-print.c 2007-07-27 12:22:09.316952092 -0400
@@ -633,7 +633,7 @@ pp_base_destroy_prefix (pretty_printer *
{
if (pp->prefix != NULL)
{
- free ((char *) pp->prefix);
+ free (CONST_CAST (pp->prefix));
pp->prefix = NULL;
}
}
diff -rup orig/egcc-SVN20070727/gcc/tree.c egcc-SVN20070727/gcc/tree.c
--- orig/egcc-SVN20070727/gcc/tree.c 2007-07-26 23:03:31.000000000 -0400
+++ egcc-SVN20070727/gcc/tree.c 2007-07-27 12:22:09.326017847 -0400
@@ -1176,8 +1176,8 @@ build_string (int len, const char *str)
TREE_CONSTANT (s) = 1;
TREE_INVARIANT (s) = 1;
TREE_STRING_LENGTH (s) = len;
- memcpy ((char *) TREE_STRING_POINTER (s), str, len);
- ((char *) TREE_STRING_POINTER (s))[len] = '\0';
+ memcpy (CONST_CAST (TREE_STRING_POINTER (s)), str, len);
+ ((char *) CONST_CAST (TREE_STRING_POINTER (s)))[len] = '\0';
return s;
}
More information about the Gcc-patches
mailing list