[Patch, c* ,ObjC*] handle string objects in format checking.

Joseph S. Myers joseph@codesourcery.com
Sun Oct 24 17:13:00 GMT 2010


On Sun, 24 Oct 2010, IainS wrote:

> 	* c-family/c-format.c (format_type): Add gcc_objc_string_format_type.
> 	(valid_stringptr_type_p): New.
> 	(handle_format_arg_attribute): Use valid_stringptr_type_p().
> 	(check_format_string): Likewise.
> 	(format_types_orig): Add NSString.
> 	* c-family/c-common.h (objc_string_ref_type_p): New prototype.

(c-family/ has its own ChangeLog.)

Is there something missing here that will be in a followup patch?

You're adding a new format style, "NSString".  There's no documentation 
for what it means, no testcases for this format style checking, and the 
table entry appears to be a dummy table entry.  What happens when the 
compiler tries to check a call to a function declared to use that type of 
format?  Are all the NULL table entries safe or will it try to dereference 
them?  Do these strings appear as STRING_CST?  If not, where is the code 
that knows how to extract the characters from them for checking?

> +/* Check that we have a pointer to a string, or string object for objc and
> +   targets that support them in c*.  */

What does "targets that support them in c*" mean?  You should explicitly 
state what the return value means, and what STRP is.

> +	      || (*targetcm.string_object_ref_type_p) ((const_tree)strp)));

Missing space in cast.

> +  if (! valid_stringptr_type_p (TREE_TYPE (type)))

Excess space in unary !.

> +  if (! argument

Likewise.

> Index: gcc/config/darwin.c
> ===================================================================
> --- gcc/config/darwin.c	(revision 165889)
> +++ gcc/config/darwin.c	(working copy)
> @@ -50,6 +50,7 @@ along with GCC; see the file COPYING3.  If not see
>  #include "debug.h"
>  #include "obstack.h"
>  #include "lto-streamer.h"
> +#include "c-family/c-format.h"

There are a few legacy c-* includes in target .c files that aren't 
c-family-specific, but you shouldn't add to them in new patches; why can't 
this go in darwin-c.c?

-- 
Joseph S. Myers
joseph@codesourcery.com



More information about the Gcc-patches mailing list