1.
I was having a discussion with somebody and I think it's just religious.
We are developing some software and he was looking through my code.
I use if (pointer) to test a pointer for NULL. He says it must be if
(p!=NULL).
They are identical. The issue is not which one you choose, but rather that it
matters at all. If either of you have a problem with one method over the other
then its that person's problem. And its a serious one -- if a person is unable
to see a trivial equivalence in their mind, then its indicative of an improper
focus or otherwise very low mental ability.
One has religious arguments about religion, not technical equivalents. The
only merit of caring is that leaving off the != NULL has fewer characters, and
thus occupies less screen space -- but its horizontal screen space which
rarely matters.
The arguments are that it's more readable and that if (p) is not portable.
"if (p)" is portable, and since it has an equivalent meaning and can't really
be mistaken to mean something else, it can't be less readable.
I said the first is a style issue [...]
Its a non-issue. They are the same thing. You lose IQ points for every second
you waste talking to your codeveloper about it. You don't have more important
issues to talk about?
2.
Then I started looking through his code and what I noticed that he very
often does things like this:
char *func(char **out)
{
*out = malloc(100);
return malloc(10);
}
This might also be a style issue, but it's a style where it's very easy to
introduce bugs (and indeed I found a few)
and even more easy to introduce bugs if you don't know the internals of the
function. So, what do you think about this style?
For me, the value of encapsulating blocks of code into functions is for the
functions to take responsibility for credibly translating input parameters to
output parameters. If a function, such as this, is essentially perform two
side-effects, it had better be worth while to do so.
I would be very suspicious of the code above, because both mallocs are not
stored in the same structure. Hence this inherently assumes that some higher
level logic is going to tie those two mallocs together which means that "func"
is not taking responsibility for tying them together. There are also 3
possible error conditions (each malloc failing, or both of them failing) all of
which have to be determined by higher level code. For the same semantics, I
would rewrite the above as:
int func (char ** out1, char ** out2) {
if (out1) *out1 = malloc (100);
if (out2) *out2 = malloc (10);
return (out1 == NULL || *out1 != NULL)
&& (out2 == NULL || *out2 != NULL);
}
In this way its more obvious that there are two outputs (since neither
parameter is decorated as "const") and the return value will tell you whether
or not there was any failure at all. Also the effect of passing in a NULL
pointer will have the effect of deactivating that output rather than simply
leading to undefined behaviour.
For someone like me, its hard to justify writing a function like the above,
however. The reason for having two mallocs *must* be because out1 and out2 are
somehow tied together possibly in the same object. Also if these are really
"char *"'s why don't you also at least initialize them with '\0's in them?
Passing around bare mallocs (i.e., where the contents are indeterminant) is
generally kind of useless.