2 style questions

[Sander]

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).
The arguments are that it's more readable and that if (p) is not portable.
I said the first is a style issue and that I think if(p) is more readable
and that the second is plain wrong.

So, is he right about the portability issue? is if (p) not strictly correct
C code?

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?

 

[Ben Pfaff]

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).

These two tests are equivalent.

The arguments are that it's more readable and that if (p) is
not portable.

The latter is not true: `if (p)' is portably equivalent to `if (p
!= NULL)' for any pointer p.

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)

It at least looks suspicious.

 

[Richard Heathfield]

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).

I ***prefer*** the latter, if(p != NULL)

The arguments are that it's more readable

I agree with this.

and that if (p) is not portable.

This, however, is nonsense.

I said the first is a style issue and that I think if(p) is more readable
and that the second is plain wrong.

You are correct. I disagree with your style choice, but it is simply a style
choice.

So, is he right about the portability issue?

No. But he's right about the style choice.

is if (p) not strictly
correct C code?

It's fine.

assert(p), however, is not (unless you have a C99 compiler).

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);
}

EEK! What is this supposed to do?

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?

I think it's ghastly. Specifically, I think the func() you showed us is
ghastly. I happen to agree with if(p != NULL) as being a very sensible way
to present the code. I prefer to reserve if(x) for situations where x is
clearly of Boolean intent, e.g.

int Split(vector *v, char *s, int WantDelimiters)
{
char *left = s;
char *right = left;
while(*left)
{
if(WantDelimiters)
{

etc

 

[Wolfgang Kaufmann]

* Thus spoke Sander <[email protected]>:

Hallo,

I use if (pointer) to test a pointer for NULL. He says it must be if
(p!=NULL).

No. But he might prefer it for readability reasons. Many programmers
that I know consider it as good programming style.

(My version of the standard is not available right now, but
| Every pointer type in C has a special value called a /null pointer/,
| which is different from every valid pointer of that type, which compares
| equal to a null pointer constant, which converts to the null pointers of
| other pointer types, and which has the value "false" when used in a
| boolean context. The /null pointer constant" in C is any integer
| constant expression with the value 0, or such an expression cast to type
| void*.
(Harbinson/Steele)

The arguments are that it's more readable and that if (p) is not portable.

I use »if(!p)« and you may further meet programmers that like to use
if(p==0), if(p==(void*)0), if(p== int*)0)...

I said the first is a style issue and that I think if(p) is more readable
and that the second is plain wrong.

As I said before I prefer the shorthand notation too, but there might be
more people that disagree on this point.

char *func(char **out)
{
*out = malloc(100);
return malloc(10);
}

So, what do you think about this style?

No.


Wolfgang.

 

[Mark McIntyre]

I use if (pointer) to test a pointer for NULL. He says it must be if
(p!=NULL).

the two are identical. I personally find the former more readable, but
the latter is just as good.

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 I don't like, because it mallocs two things, and doesn't make it
terribly clear its doing either.

 

[Malcolm]

So, is he right about the portability issue? is if (p) not strictly correct
C code?

if(p) is more expressive. For someone who doesn't know C, if(p != NULL) is
more understandable, but you can't go far in C without knowing that non-zero
equates to true and NULL = (==) 0 in C programs.

char *func(char **out)
{
*out = malloc(100);
return malloc(10);
}

So, what do you think about this style?

There's no easy solution to this. When you need to return more than one
value you have to either use a structure or use pointers. Where those values
are arrays allocated with malloc() things become messy, since you've got to
document that they have to be freed by the caller.

void func(char **out1, char **out2)

would have the benefit of showing that the two outputs were equivalent.
Similarly you would write

void getcursorpos(int *x, int *y)

int getcursorpos(int *y)

However if one value is the major return of the function, whist another is
subsidiary, then maybe the original style makes sense.

Eg in an adventure game

char *getnextroomdesc(char **error)

where very occasionally it will return 0 and set error.

 

[Sander]

char *func(char **out)

There's no easy solution to this. When you need to return more than one
value you have to either use a structure or use pointers. Where those values
are arrays allocated with malloc() things become messy, since you've got to
document that they have to be freed by the caller.

Ai, I was not very clear about the second.
It's not about returning two values, it's about returning dynamic memory
from a function.
Sometime he does something like this:
char *fun(void) { return malloc(10); }

and sometimes this:
void fun(char **s) { *s = malloc(10); }

I consider this very bad style, to the point that it's really crappy
unexperienced C coding.

 

[Kalle Olavi Niemitalo]

if(p) is more expressive. For someone who doesn't know C, if(p != NULL) is
more understandable, but you can't go far in C without knowing that non-zero
equates to true and NULL = (==) 0 in C programs.

In a program that I develop, there are several types of handles
with special null values. For example, a variable of type
msg_handle_t can have the special value MSG_HANDLE_NULL, which
means the handle does not refer to any message. However,
MSG_HANDLE_NULL is defined as 0 on one platform and 0xFFFF on
another, so "if (handle)" would be a bug; I must use "if
(handle != MSG_HANDLE_NULL)" instead. Out of habit, I then use
"if (pointer != NULL)" as well.

 

[Mark McIntyre]

It's not about returning two values, it's about returning dynamic memory
from a function.

absolutely nothing wrong with that. Useful technique on occasions -
imagine you're loading a buffer with data from somewhere (disk,
network, whatever), the caller can't know how much memory to allocate,
so get the callee to do it. The alternative is to repeatedly call the
callee, which is not CPU friendly.

Sometime he does something like this:
char *fun(void) { return malloc(10); }

and sometimes this:
void fun(char **s) { *s = malloc(10); }

Well, its inconsistent to mix styles, but either is ok. Personally
I'd prefer to pass in the pointer, and return a status instead of
void.

I consider this very bad style, to the point that it's really crappy
unexperienced C coding.

On the contrary, its a well established technique. What it /does/
require is more care in your documentation and in freeing the memory
afterwards. So its a more *dangerous* technique. But as they say, its
a poor atom blaster that doesn't point both ways.

 

[Sidney Cadot]

It's fine.

assert(p), however, is not (unless you have a C99 compiler).

It isn't? This is a genuine surprise... Why is this?

Best regards,

Sidney

 

[Sander]

On the contrary, its a well established technique. What it /does/
require is more care in your documentation and in freeing the memory
afterwards. So its a more *dangerous* technique. But as they say, its
a poor atom blaster that doesn't point both ways.

It's certainly more dangerous and I already found a mem leak in his own
code.
I wonder what would happen if I started using that code.

 

[CBFalconer]

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).
The arguments are that it's more readable and that if (p) is not portable.
I said the first is a style issue and that I think if(p) is more readable
and that the second is plain wrong.

So, is he right about the portability issue? is if (p) not strictly correct
C code?

I agree with you. Both styles are portable.

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?

Very ugly. I cannot conceive of a legitimate use for the result.
I can conceive of several illegitimacies.

 

[nrk]

It isn't? This is a genuine surprise... Why is this?

Best regards,

Sidney

assert works only on expressions of integer type in C89, and results in UB
otherwise. So, you should write assert(p != 0) or assert(p != NULL).
However, in C99, it takes expressions of any scalar type, so assert(p) is
fine.

-nrk.

 

[Richard Heathfield]

It isn't? This is a genuine surprise... Why is this?

In C90, assert takes an expression of int type. If p is of some pointer
type, then the expression

p

has pointer type, not int type.

The expression

p != NULL

has int type, and is thus suitable for use in assert(). As I have already
hinted, this changed in C99, so that assert takes a scalar expression.
Therefore, in C99, assert(p) is correct (albeit not as readable IMHO as
assert(p != NULL), but that's a style issue).

 

[The Real OS/2 Guy]

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).

Ask him why he doesnt use
if ((p != NULL) != 0) != 0)
or better
if (p == (0 == 0 && 0 == 0x00 && 0 == NULL && 0 == 000))
or
if (p != (NULL - 0))
or
if ((p + 0) != (0*0))
or
if (!p == !NULL)
or......

because when he needs a superflous compare he should never stp by one
but by an infinite number of compares to get it.

It seems he has never learnd programming in C.

Ask him to name the point in the standard that forbits if(p).

Ask him too why he does thing that if(p) should give something
different than
if ((p + 0 - '0' - '0') != ('0' - '0' + 0)

or any other constrct that needs more to type.

C has lots of possibilitzies to spend useless keystrokes to get
something written - but even lots of shorthands to make something easy
to type and to understund.

The arguments are that it's more readable and that if (p) is not portable.

Shows only that he knows absolutely nothing about he tells. Tell him
he should learn programming in C before he starts to tell about.

I said the first is a style issue and that I think if(p) is more readable
and that the second is plain wrong.

You're right that if(p) is equal to if (p == (0 == NULL == 0x00 == 000
== (0+0))

So, is he right about the portability issue? is if (p) not strictly correct
C code?

He is absolutely wrong - not only for compatibility but for
readability too. Tell him C is NOT C64 BASIC where you have no chance
to make tests for 0/NULL/zero/'\0' clean and short.

He looks like a beginner who has never learned right C.

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);
}

Hm, that's a method to allocate 2 different, independant areas of
memory. Iteresting is if he is checking both results of that fuction
for success or if he assumes errornousely that this method would
always return both or none requested areas. There are possibilites
that both, one or none of the malloc() may fail. This kond of function
does nothing than to hide that there are more than one memory area
gets allocated. Looks at least as a fine source for creating memory
leaks.

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?

You're compleately right.

 

[The Real OS/2 Guy]

I ***prefer*** the latter, if(p != NULL)


I agree with this.

So you should write

if ((p != NULL) == !TRUE)

Is at least more readable as it clearly thays that the condition is
true when the compare to NULL is true. But then you shoud write for
more clarity

if (((p != NULL) == !TRUE) == !TRUE)

But then why stop by the 3 compares? Increase it to an unlimited
amount of compares to get it really clear. Amny != is superflous but
confues to expoermented C programmmer in first step. if (p) is sowhat
of clear because ther is no need to compare the test result with
something. The test itself is anthing needed

This, however, is nonsense.


You are correct. I disagree with your style choice, but it is simply a style
choice.


No. But he's right about the style choice.


It's fine.

assert(p), however, is not (unless you have a C99 compiler).

assert() is ALWAYS a source of confusing the user and destroying data
consistency! assert is NOT designed to test data! It is only designed
to test the program itself to catch programming errors. For testing
errors on data use the if or the ?: operator. Do the approciate
actions in case of error - but don't use assert.

assert() is originated to catch errors a programmer may do. For that
it is fine to let the program crash at the earliest possible moment
because that makes it more simple to dedect the programming error -
even as the data the program handles may get destroyed. But for that
you're in the debug phase.

I think it's ghastly. Specifically, I think the func() you showed us is
ghastly. I happen to agree with if(p != NULL) as being a very sensible way
to present the code. I prefer to reserve if(x) for situations where x is
clearly of Boolean intent, e.g.

int Split(vector *v, char *s, int WantDelimiters)
{
char *left = s;
char *right = left;
while(*left)
{
if(WantDelimiters)
{

etc

if (p) means 'is p a valid pointer then ....'

if (!p) means 'is p an invalid pointer then ....'

What can be more clean as an self documenting statement?

if (p != NULL) means 'if p is not NULL' That is an question nobody
asks really.

 

[The Real OS/2 Guy]

Ai, I was not very clear about the second.
It's not about returning two values, it's about returning dynamic memory
from a function.
Sometime he does something like this:
char *fun(void) { return malloc(10); }

Oh, its really fun-ny for a maintenancer to get all the hidden malloc.

and sometimes this:
void fun(char **s) { *s = malloc(10); }

I consider this very bad style, to the point that it's really crappy
unexperienced C coding.

Tell him that he should increas the number of different interfaces to
do the same thing. It makes the maintenancer happy to find all the
different interfaces to get exactly the same thing done. He can do
that by creating functions that does not only accept pointer to
pointers but pointers to pointers to pointers..... and return values
only that way, because it is always irretating to find functions not
returning directly but using pointers to pointers...... It will make
maintenance so easy to have only functions returning void but using
parameters to get results back.

Tell him that the best way is always NOT to use arithmetic operators
like +,-,*,/ but use functions that receive pointers to the values
they have to read and pointers to pointers to pointers to give the
value of the simple operation back.

A simple wrapper auround malloc is too easy. Tell him he should write
wrapper functions around the functions and then wrappers around the
wrappers for each statement he has to produce. This will make
maintenance a bit more complicated as he tries to do yet.

To get the final state of confusing maintenancers he should use
pointers to pointers to pointers of functions and use only them to
build the call tree to get each single statement done. As this makes
maintenance so asy he will save his job for the next thoused years
because there will be nobody able to maintenance his code.

 

[Jens =?ISO-8859-15?Q?Pr=FCfer?=]

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).
The arguments are that it's more readable and that if (p) is not portable.
I said the first is a style issue and that I think if(p) is more readable
and that the second is plain wrong.

Hello,

You are right about the portability issue. Of course both versions are
perfectly portable. Now I am only an occasional C programmer but I have
learned to put the constant first in comparisons, because especially when
testing for "equals", it makes the compiler complain when you are using "="
instead of "==". Example:

if (CONST == a) ...

and not

if (a == CONST) ...

The easy made error

if (a = CONST)

can be nasty since it always returns TRUE, while CONST = a gives you an
"lvalue" error.

So I would go with your version of just using if(p). Going with his, I'd
change to if (NULL != p).

Just my two cents

Cheers

Jens

 

[Mac]

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).
The arguments are that it's more readable and that if (p) is not portable.
I said the first is a style issue and that I think if(p) is more readable
and that the second is plain wrong.

So, is he right about the portability issue? is if (p) not strictly correct
C code?

Others have explained that "if (p)" and "if (p != NULL)" are functionally
equivalent. I prefer to write if "(p != NULL)" because in my situation I
cannot assume that everyone who reads the code is an experienced C
prorgrammer. I myself have seldom been paid directly to produce C code.
Usually the code is incidental to something else, or written just for fun.

When I am reading code, if(p) doesn't bother me in the least. The only
problem would be if it were in a context where it was difficult to find
out what type p is. This is an unlikely scenario, and anyway, most
programming oriented editors let you jump right to the declaration.

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);
}

I'm not sure what you mean.

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?

On an unrelated note, it sounds like you have major issues with this
programmer. If your goal is to get along with him/her, I'm not sure that
proving you are right and that, in addition, his/her code sucks, is going
to help you reach your goal.

Programming is easy. Finding the right balance between accomodation and
stubborness is hard, particularly when you are right.

Mac

 

[xarax]

Sander wrote:

/snippage about if(p) versus if(p != NULL)/

You are right about the portability issue. Of course both versions are
perfectly portable. Now I am only an occasional C programmer but I have
learned to put the constant first in comparisons, because especially when
testing for "equals", it makes the compiler complain when you are using "="
instead of "==". Example:

if (CONST == a) ...

and not

if (a == CONST) ...

The easy made error

if (a = CONST)

can be nasty since it always returns TRUE, while CONST = a gives you an
"lvalue" error.

So I would go with your version of just using if(p). Going with his, I'd
change to if (NULL != p).

Just my two cents

Cheers

Jens

Yes, for non-pointers, I generally put the constant on
the left and the variable on the right. This is a religious
style issue, similar to where braces are put (at the end
of the line or on a line by itself), so expect some flames.
I adopted that style after getting burned by the typo error
"if(variable = constant)". I haven't looked back since and
have no regrets.

Years ago, I had an instructor that spent a few minutes
"fixing" my code when I asked an unrelated question, so that
he could then proceed to read my code and answer the question.
He changed every occurance of "constant == variable" to
"variable == constant". I asked what difference it made and
he sheepishly acknowledged that it made no difference.

For pointers, I use "if(p)" or "if(!p)". It is simple and
concise. In my mind, I understand that "if()" is testing
for non-zero versus zero, and that zero is the canonical
representation of a NULL pointer value. Folks who don't
understand that simple concept are not allowed anywhere
near my code.


--
----------------------------------------------
Jeffrey D. Smith
Farsight Systems Corporation
24 BURLINGTON DRIVE
LONGMONT, CO 80501-6906
http://www.farsight-systems.com
z/Debug debugs your Systems/C programs running on IBM z/OS!