Domain of the Bored

At long last, a new version of Everything you need to know about pointers.

The most significant changes are long-overdue corrections regarding declarations of const pointers and the difference between arrays and pointers. You can—and, if you learned how to work with pointers from this tutorial, should—read all of the changes in the delta between 1.2 and 1.3.

I've started turning on most of Xcode's warning options and one warning-related build setting in all of my personal projects. I suggest you do the same.

There are some warnings I don't turn on, for any of several reasons:

• They're inapplicable. For example, “‘Effective C++’ Violations” doesn't apply to me, because I don't write C++ code.
• They don't help anything. An example is “Four Character Literals”, which is about 'abcd' literals for four-byte-code types such as OSType. These sacrifice something convenient for no benefit, so I leave them off.
• They are impossible for me to fix. An example is “Multiple Definition Types for Selector”. Cocoa raises that one all over its headers, and I can't do anything about Cocoa.

The rest of the warnings, I turn on because either they make something clearer or they tell me about either real or potential (i.e., future real) bugs. They are:

• Check Switch Statements

  Warn whenever a switch statement has an index of enumeral type and lacks a case for one or more of the named codes of that enumeration. The presence of a default label prevents this warning.

  Leave no case unhandled.

  Consider whether a default: label is appropriate for your enumeration. If your switch statement handles all possible values, cut out the default and assert that the value is one of the possible ones instead. An easy way to do this, if the enumeration values are serial and the enumeration is not part of an API you expose, is to have one extra name defined as the number of possible values:

  enum Foo {
  	kFooFoo,
  	kFooBar,
  	kFooBaz,
  	kFooNumberOfValidFooValues
  };
  

  Then, in your assertion macro, compare the value against that name:

  #define MyParameterAssertValidFoo(foo) \
  	NSAssert1((foo) < kFooNumberOfValidFooValues, @"Invalid Foo value: %d", (foo));

  When you add kFooQux, insert it above kFooNumberOfValidFooValues, and the value of kFooNumberOfValidFooValues will increase by one to fit it.

  The result is that your switch statement covers all known values for the enumeration (or you get a warning because it doesn't), and your method throws an exception (from the assertion) whenever anyone passes an unknown value.

• Hidden Local Variables

  Warn whenever a local variable shadows another local variable, parameter or global variable or whenever a built-in function is shadowed.

  One common way to get this warning is to name a variable index, because there is a function by that name in the standard C library. That's not as much of a false positive as you may think: If you fail to declare your index variable, all your references to it will actually refer to the index function. You can see how it would be bad to send a message such as [myArray objectAtIndex:index] with this bug.

  The solution is simple: Never, ever name a variable index.

• Implicit Conversion to 32 Bit Type

  Warn if a value is implicitly converted from a 64 bit type to a 32 bit type.

  This is most useful when converting old code to work correctly in a 64-bit architecture. Storing a pointer into an int variable (such as a reference constant) when targeting an LP64 architecture is a good way to get this warning, and rightly so.

• Initializer Not Fully Bracketed

  Example, Here initializer for a is not fully bracketed, but that for b is fully bracketed.

  	int a[2][2] = { 0, 1, 2, 3 };
  	int b[2][2] = { { 0, 1 }, { 2, 3 } };

  This is a cleanliness warning. It also applies to structures, such as NSRect:

  NSRect warns = { 0.0f, 0.0f, 640.0f, 480.0f };
  NSRect doesNotWarn = { { 0.0f, 0.0f }, { 640.0f, 480.0f } };

  (In real code, I'm more likely to use NSZeroPoint instead of the { 0.0f, 0.0f } element above. It's harder to spell that wrong and get away with it than it is to get away with typing 9.9f, 1.1f, or 2.2f instead of 0.0f.)

• Mismatched Return Type

  Causes warnings to be emitted when a function with a defined return type (not void) contains a return statement without a return-value. Also emits a warning when a function is defined without specifying a return type.

• Missing Braces and Parentheses

  Warn if parentheses are omitted in certain contexts, such as when there is an assignment in a context where a truth value is expected, or when operators are nested whose precedence people often get confused about.

  Also warn about constructions where there may be confusion to which if statement an else branch belongs. Here is an example of such a case:

  	if (a)
  		if (b)
  			foo ();
  	else
  		bar ();

  In C, every else branch belongs to the innermost possible if statement, which in this example is if (b). This is often not what the programmer expected, as illustrated in the above example by indentation the programmer chose.

  This may appear to be just a cleanliness warning, but as you can see from the example, it can also warn you about code that may not flow the way you expect it to.

• Missing Fields in Structure Initializers

  Warn if a structure's initializer has some fields missing. For example, the following code would cause such a warning, because "x.h" is implicitly zero:

      struct s { int f, g, h; };
      struct s x = { 3, 4 };

  This option does not warn about designated initializers, so the following modification would not trigger a warning:

      struct s { int f, g, h; };
      struct s x = { .f = 3, .g = 4 };

  I'm not sure why it warns about the former and not the latter, since all the members get initialized in both code examples (C99 §6.7.8 ¶21). If nothing else, this warning is good motivation for you to switch to designated initializers, which make your code more explicit about which members it's initializing.

• Missing Newline At End Of File

  Another cleanliness warning—this one, about the cleanliness of diffs.

• Sign Comparison

  Warn when a comparison between signed and unsigned values could produce an incorrect result when the signed value is converted to unsigned.

• Strict Selector Matching

  Warn if multiple methods with differing argument and/or return types are found for a given selector when attempting to send a message using this selector to a receiver of type "id" or "Class". When this setting is disabled, the compiler will omit such warnings if any differences found are confined to types which share the same size and alignment.

  I don't turn this one on, because it's unnecessary. When the multiple declarations differ significantly (e.g., one method returns an object and the other returns a float), the compiler will raise the warning whether it's turned on or not. When the declarations don't differ significantly (e.g., both methods return an object), the difference won't cause a problem, so you don't need to worry about it.

  So, you should leave this one off.

• Typecheck Calls to printf/scanf

  Check calls to printf and scanf , etc, to make sure that the arguments supplied have types appropriate to the format string specified, and that the conversions specified in the format string make sense.

  The biggest reason to turn this on is that it checks your use of methods that take a nil-terminated list of arguments:

  NSArray *array = [NSArray arrayWithObjects:@"foo", @"bar"];

  That message should have a nil after the last argument. With this warning turned on, the compiler will point out that I don't.

  The ostensible main reason to turn this on is to have the compiler check your uses of printf and scanf formats. I don't use printf often (and I never use scanf), so that's not so important for me, but when I do, this could come in handy.

  Sadly, it doesn't work on NSLog calls.

• Undeclared Selector

  Warn if a "@selector(...)" expression referring to an undeclared selector is found. A selector is considered undeclared if no method with that name has been declared before the "@selector(...)" expression, either explicitly in an @interface or @protocol declaration, or implicitly in an @implementation section.

  Another benefit of this warning is that you can use it to get a warning when you pass a wrong key to a KVC, KVO, KVV, or Bindings method. Uli Kusterer has a macro for that.

• Unused Functions

  Warn whenever a static function is declared but not defined or a non-inline static function is unused.

  Works best with a policy of declaring any function as static that you don't need to be visible elsewhere in your program.

• Unused Labels

• Unused Values

• Unused Variables

  These follow the general rule of “code you don't have is code you don't have to debug”. If you're not using a label, expression statement, or variable, you don't need it, and you will find your code clearer without it.

  You may notice that I don't turn on Unused Parameters. Most times when I trip that warning, it's a callback function or method, so I can't get rid of the argument. Rather than litter my code with bright yellow #pragma unused(foo) directives, I prefer to just turn this one off. (See my rule above about less code being better.)

Once I have turned on all of these warnings and then eradicated them from my code, I turn on two more build settings:

• Treat Warnings as Errors

  I call this “hardass mode”.

  Remember what I said above: Almost all of these warnings represent real or potential (i.e., future) bugs in my program. Rather than tolerate them, I turn this setting on so that any time I write such a bug, I break the build.

  I haven't been able to turn this on yet in Adium or Growl, although I have turned it on in Adium's Spotlight-importer project. I do, however, turn it on in all of my solo projects.

• Run Static Analyzer

  Activating this setting will cause Xcode to run the Clang static analysis tool on qualifying source files.

  The Clang Static Analyzer is the find-your-bugs-for-you tool you've heard so much about. This setting makes Xcode run it whenever you build. Thus, every build, you get all those warnings errors and your analysis results.

  Whenever possible, I leave this on; if there's a source file that it takes a long time to analyze (e.g., GrowlPluginController.m), then I turn it off, but only then.

UPDATE 2009-11-22: Jonathan “Wolf” Rentzsch wrote a script to turn on all of these settings in all of the projects you have open.

UPDATE 2009-11-28: Updated the entry on “Typecheck Calls to printf/scanf” after seeing that Jeremy W. Sherman pointed out a much better benefit of it in a comment on a Stack Overflow answer.

UPDATE 2009-12-05: Corrected the discussion of the index problem. You can't use index, or any other function, as a C-array subscript, so the problem only affects higher-level arrays, such as NSArray.

This is mainly for Andy Finnell on Twitter, who wonders why some of us avoid naming variables index.

I pointed out that there is a function in standard C named index, and this causes one of two problems: If you declare a variable named index, you have shadowed the function and should get a warning for that; if you fail to declare the variable, you pass the pointer to the index function as your array index, which is probably not what you intended.

I say “should” there because, as he noted in his response, the shadowed-name warning is off by default. You should turn it on, because it catches bugs. In fact, the index bug is one that it can prevent.

Suppose you do name a variable index, and either you don't have the shadowed-name warning turned on or you ignore it. You initialize the variable with an index, but don't otherwise assign to it. Then, you attempt to access an object in an array by this index.

All well and good so far. index is a variable, so everything works as intended.

But then, one of several things happens:

1. You comment out both the declaration and the usage of index, for whatever reason, but then you uncomment the usage but forget to uncomment the declaration.
2. You update and/or merge in your version-control system, or otherwise apply one or more diffs. Usually, this works, but today isn't your lucky day: The merge breaks your source code. Perhaps it introduces conflicts, and you resolve them incorrectly. Or maybe it breaks the code silently (e.g., by merging in another branch's division of this function into two).
3. You move the code to another location, but you forget to move half of it, or you move one half and delete another, forgetting that the declaration of index was in the code you deleted.

You had a variable named index, but now you don't—but the index function is always there*. Since there is something named index, your code compiles. It's the wrong type, so you'll get a warning, but maybe you don't notice it.

Then you run the code and it crashes. Why? Because you passed a function as the index into an array.

In the worst possible case, it was #2 and you weren't aware that this code was affected. Maybe you'd been working on something else. Anyway, since you hadn't been working on the now-broken code, you aren't testing it**, so you don't know that it's now broken.

So you ship it. You ship this index-way-out-of-range crasher. And then your user runs the code and gets the crash.

This isn't theoretical. I've had this happen more than once (fortunately, not in the hands of a user). It's one reason why I turn on the shadowed-name warning and “Treat Warnings as Errors”, and it's the reason why I never use index as a variable name.

UPDATE 2009-12-05: To clarify, this problem does not affect C arrays, as C does not allow you to use a pointer in an array subscript. It mainly affects higher-level array interfaces, such as Cocoa's NSArray.

* Assuming that, directly or indirectly, you've included string.h. If you're using Cocoa, you have (Core Foundation includes it). ↶

** Unless, of course, you have automated test cases covering this code, and you run those. ↶

Before:

if (![newTrackURL isEqualToString:trackURL] || ([newTrackURL hasPrefix:@"http://"] && !([lastPostedDescription isEqualToString:displayString]))) { // this is different from previous notification, or it's a stream and the playing track has changed

After:

BOOL URLChanged = ![trackURL isEqualToString:newTrackURL];
BOOL isStream = [newTrackURL hasPrefix:@"http://"];
BOOL descriptionChanged = ![lastPostedDescription isEqualToString:displayString];
if (URLChanged || (isStream && descriptionChanged)) {

Why remove the comment? Well, I had just changed the code to what you see in the Before example, but I had not changed the comment, and I thought I was ready to commit. I almost committed a stale comment. It's that easy. Luckily, I caught it in reading the patch before I qfinished it, so I was able to change the comment to what you see above.

Now I have no comment at all, but the code is clear, so I don't need one. This is an improvement.

Consider this code:

enum { foo, bar, baz } my_var;
switch (my_var) {
    case foo:
        int foo_alpha; //Line 7
        int foo_beta;
        break;

    case bar:
        my_var = baz;
    case baz:
        printf("Bar or baz encountered\n");
        break;
}

Try to compile it as C99.

Here's what GCC says:

test.c: In function ‘main’:
test.c:7: error: syntax error before ‘int’

What!

First off, why not? What is invalid about this declaration statement?

Second, why is foo_alpha invalid and not foo_beta?

There are several definitions in the C99 specification that come together to cause this problem.

The first is that there is no such thing as a declaration statement, because declarations are not statements. See §6.7 and §6.8; note that neither definition includes the other. In the language that is C99, declarations and statements are separate concepts.

The second is the definition of a compound statement. The definition of a switch statement (which is part of §6.8.4) is:

switch ( expression ) statement

If you go back up to §6.8, you'll see that another possible kind of statement is a compound statement, for which §6.8.2 gives this definition:

compound-statement:

• {   block-item-list_opt   }

block-item-list:

• block-item
• block-item-list   block-item

block-item:

• declaration
• statement

So a declaration is not a statement, a compound statement can contain declarations and/or statements, and a switch statement is a prefix upon (usually) a compound statement.

Now, the kicker. Read the relevant definition of a labeled statement from §6.8.1:

case   constant-expression   :   statement

Statement. Not block-item. Not declaration. Statements only.

So this is what the compiler sees in valid code (with a declaration not following a case label):

• selection statement (switch)
  • compound statement
    • labeled statement
      • statement
    • statement
    • statement
    • declaration
    • statement
    • jump statement (break)
    • labeled statement
    • statement
    • jump statement (break)
    • labeled statement
    • statement
    • jump statement (break)

Now, consider how the compiler sees my code above:

• selection statement (switch)
  • compound statement
    • labeled statement
      • declaration — wait, this isn't a statement! ERROR
    • declaration — also a kind of block-item, so it's perfectly valid here
    • jump statement (break)
    • labeled statement
    • statement
    • labeled statement
    • statement
    • jump statement (break)

I hope this makes clear that this isn't a compiler bug; the C99 language really does work this way.

(One possible solution would be to make a declaration a kind of statement, but I don't know what other ramifications that might have. [UPDATE 11:36: Jeff Johnson tells us why not.])

New series: Manpage FridayMonday. Every Fridayother Monday, I will post a link to one manpage that comes with Mac OS X. [See update below.]

Today, it's backtrace(3), which tells you about three functions:

SYNOPSIS

#include <execinfo.h>

int backtrace(void** array, int size);

char** backtrace_symbols(void* const* array, int size);

void backtrace_symbols_fd(void* const* array, int size, int fd);

DESCRIPTION

These routines provide a mechanism to examine the current thread’s call stack.

backtrace() writes the function return addresses of the current call stack to the array of pointers referenced by array. At most, size pointers are written. The number of pointers actually written to array is returned.

backtrace_symbols() attempts to transform a call stack obtained by backtrace() into an array of human-readable strings using dladdr(). The array of strings returned has size elements. It is allocated using malloc() and should be released using free(). There is no need to free the individual strings in the array.

backtrace_symbols_fd() performs the same operation as backtrace_symbols(), but the resulting strings are immediately written to the file descriptor fd, and are not returned.

Posted in C, Documentation, Mac OS X, Manpage Monday, Programming | 3 Comments »