Is there good argument checking strategy?

Question

instead of checking every input pointer argument with if condition if( in != NULL ), is there better way to do this. One method is to create some macros, like

#define CHECK_ARG(x) if(!(x)) { return 0; }

In code it's look like this

int some_func( int * first_arg, int * second_arg )
{
    CHECK_ARG( first_arg );
    CHECK_ARG( second_arg );

    // some operations
    //
}

In my opinion this is good approach, but before adding this, I want to know is there any better ways doing this?

Thanks in advance!

What will you gain with the macro? I don't think it helps to make the code more readable. The opposite I'm afraid. You could use `if ( !first && !second ) return 0;` — George Kastrinis, Aug 28 '11 at 12:21
What if som_func has another return type, for example, void? — Alex F, Aug 28 '11 at 12:22
@Alex, in my code every function which needed in this kind of checking return 1 or 0. I think it's not correct to make function void if input argument could be NULL. — akmal, Aug 28 '11 at 12:32
@George, the macros I showed, is the simplest, in my code, in the case of error, I'm writing error message to log file, and return error code. So I'll add one more argument to this macros, number of argument. The reason of creating this macro is, if my function has 3 or 4 pointer arguments, I have to add 3 or 4 if condition, which makes my code messy. — akmal, Aug 28 '11 at 12:37
@George Kastrinis, read carefully, I wrote that this macros also must write to log file and set error code. What I can do in this case? `if ( !first && !second && !third && !fourth ) // in this case I can't create valuable message for user ` — akmal, Aug 28 '11 at 12:44
This question has an unreasonable assumption that functions are somehow responsible for accepting a certain invalid pointer and assigning it a special meaning. Of course there's nothing they can do about the billions of other invalid pointer values. STOP CHECKING FOR NULL. It's wrong, unless your function's documentation specifies that it accepts NULL as a special case and does something special when you pass NULL. — R.. GitHub STOP HELPING ICE, Aug 28 '11 at 14:25
@R I agree. However, if the function is at an API boundary and the API uses NULL in other cases, checking for it in functions where it is not allowed might be a worthwhile defensive programming technique. — Diomidis Spinellis, Sep 02 '11 at 15:24
@R.. Here is an example from the Thunderbird source code: `NS_IMETHODIMP nsPlaintextEditor::GetWrapWidth(PRInt32 *aWrapColumn) { if (! aWrapColumn) return NS_ERROR_NULL_POINTER; *aWrapColumn = mWrapColumn; return NS_OK; }` — Diomidis Spinellis, Sep 03 '11 at 15:22
@Diomidis: I've discussed this issue to death here: http://stackoverflow.com/questions/4390007/in-either-c-or-c-should-i-check-pointer-parameters-for-null/4390210#4390210 It's my belief that checking for NULL and trying to return an error leads to buggier code than just letting the program crash, the basic reason being that a programmer who is already violating contracts is unlikely to be performing correct return value checking. — R.. GitHub STOP HELPING ICE, Sep 03 '11 at 15:46

score 6 · Answer 1 · answered Aug 28 '11 at 12:21

6

I would use some ASSERT in there instead - at least in your debug version.

Also, what George said - its not readable and clear that it returns 0.

answered Aug 28 '11 at 12:21

Daniel A. White

187,200
47
362
445

As you write ASSERT is good for debug version, but I'm creating release version. Macros I showed, is only idea, it is not ready yet. Ready version must write to log file, and set error code. – akmal Aug 28 '11 at 12:40

score 2 · Answer 2 · answered Aug 28 '11 at 12:22

Macros are usually frowned upon, but sometimes extermely useful. But macros that have "hidden" side effects (like returning) are potentially dangerous, because they're difficult to spot. At least consider renaming your macro to make this explicit, e.g. CHECK_ARG_AND_RETURN_IF_INVALID. It's verbose, but it's hard to miss.

Diomidis Spinellis · Accepted Answer · 2011-08-28T13:03:34.707

In your function write

assert_or_return(first_arg != NULL);
assert_or_return(second_arg != NULL);

Define assert_or_return so that in the debug version it will fire up the debugger (by calling abort), and in the release version it will log the error and return (as you want to do). I assume you're confident that returning from the function when one of the arguments is NULL will do more good than harm in the production version of your application. If by returning early you will irrevocably corrupt your users' data or cause a furnace to explode, it would be better to terminate early, perhaps with a custom error message.

The following example illustrates how your idea can be implemented.

#ifdef NDEBUG
#define assert_or_return(expr) do { \
  if (!(expr)) { \
    fprintf(logfile, "Assertion %s in %s(%d) failed\n", #expr, __FILE__, __LINE__); \
    return 0; \
  } \
} while(0)
#else
#define assert_or_return(expr) do { \
  if (!(expr)) { \
    fprintf(stderr, "Assertion %s in %s(%d) failed\n", #expr, __FILE__, __LINE__); \
    abort(); \
  } \
} while(0)
#endif

Let's I show one of my functions, `CopyFolderToMedia( const wchar_t * srcPath, const wchar_t * dstPath );` When srcPath or/and dstPath is NULL, I must stop executing, and inform user that he passed wrong argument. Does my approach is right? — akmal, Aug 28 '11 at 12:55
The do { ... } while(0) construct is needed to ensure the macro doesn't mess up the parsing of a conditional statement. It's a standard defensive programming practice. — Diomidis Spinellis, Sep 02 '11 at 15:25

score 0 · Answer 4 · answered Aug 28 '11 at 12:23

0

Maybe you can assertions. Have a look at assert.h.

answered Aug 28 '11 at 12:23

Reto Aebersold

16,306
5
55
74

see my comment for Daniel A. White's answer – akmal Aug 28 '11 at 12:49

Is there good argument checking strategy?

4 Answers4