When to check the return value of malloc

S

Seebs

No, I didn't have a more exact term to describe it, and since C does not
allow overloading, I thought that it might be descriptive in this case.
Hmm.

The fact is that you are using p in two different ways to send two
different kinds of information. This isn't really all that different from
the ways many standard functions like fgetc() return an int for what is
expected to be a char value so that they can also send error information
along with any values. This is questionable practice that tends to cause
lots of trouble (in this case particularly to newcomers.)

Oh, certainly it's questionable. Go ahead and question it.
In general, it is a much better practice to explicitly separate the data
outputs. The structure is sent in channel while the error code is sent
separately out of channel. If I was going to write a function that creates
p, I often use something like this:

I am not sure this is actually that much better a practice, especially in C,
where it's pretty much idiomatic to use a null pointer to indicate "we don't
have a foo".
/*
creates a valid printer_dev at the buffer pointed to by printer -- returns
SUCCESS or FAILURE depending on whether a valid pring_dev could actually be
created
*/
fulfillment create_printer(printer_dev* printer);
In this way, I have an explicitly differentiated the error information from
the data itself.

And also forced the user to know how to allocate a printer, which does nothing
but push the issue somewhere else. Now the user has to write:

printer_dev *p = get_new_printer();
if (p)...

So you're right back where you started.

You can, of course, do:
fulfillment create_printer(printer_dev **printer);
and declare that, if it doesn't return SUCCESS, it hasn't done anything to
"printer".

But that's annoying! Furthermore, even if you've done this, you STILL have
the possibility that "printer" will be a null pointer.

You can never eliminate the possibility of a pointer being null-or-invalid
unless it's the address of a static object. For most real-world cases,
dynamic allocation is necessary. You can't detect an invalid pointer, so
you have to use null as the "we don't have one" state.

Since you MUST do that, it is ridiculous to then attach a second status that
isn't the pointer's status, because the moment you do that, you've created
two pieces of information which are supposed to reflect the same thing, and
thus the possibility for them to be out of sync.
As somebody pointed out, there are
different reasons why the printer may not be available. NULL only
allows you to say that it isn't available; but, provides no information as
to why.

Right, because at the point where you'd be *using* the printer, you don't
care why. It's only at the point that an attempt was made to *allocate*
a printer that you care why, and the error handling for that belongs
there.
Not really. There may be several layers on top of the extra printer
handling functionality.

That would be "much, much, harder to arrange". You're creating bloat in
code, data storage, and layers of abstraction, all to avoid something that
is clear, idiomatic, and useful.
Agreed, but the code that calls do_printer still must be able to handle the
differing ways in which do_printer might fail.

Yup. But "no printer available" is plenty complete for this point; the
point at which a more specific problem with printer availability would
have been addressed would have been when p was configured.
As I have already said, a NULL pointer indicates that there is an error.
It doesn't explain why.

It doesn't have to. The "why" was addressed when the pointer failed to
acquire a non-null value. Past that, all you need to know is that there
is not currently a printer.
Even with an absence of requirements that different
errors handled differently, it is much more descriptive to say:

if (is_printer_available())
/* ... */
else
/* error code */

then:

if (printer != NULL)
/* ... */
else
/* error code */

The second only tells me that there is a problem. The second tells me that
there is an error because no printer is available.

They're completely identical. Both tell you that there is no printer
available. If there were a printer available, printer would not be a null
pointer.
You mistaken in my reasoning. Likely, the test will be performed much
higher up before the system even attempts to generate information to send
to the printer. It makes no sense to do this much if there is not going to
be an printer to send it to. Secondly, the test will likely only need to
be in a single place inside of the function interface that actually
initiates any printer operations.

This seems very speculative, and you've repeatedly talked about adding
additional layers of interfaces, which sounds like a lot of overhead to
satisfy a prejudice.

In C, a null pointer is idiomatically used to indicate the unavailability
of a thing that would otherwise have been pointed to. That's the idiom
the language standard uses, for everything from malloc() to fopen(). Using
any other idiom is gratuitously confusing.
First see above, since test only needs to be called at the first common
entry into the printing system it is no less robust.

I don't buy this notion of a "first common entry into the printing system".
The printing system is used by the rest of the program; every part of the
rest of the program that might wish to print has to know how to tell what
it can or can't do.
Second, while allowing an error to propogate through the code, there are
many more places that it could fail.

Not really. It's just as practical to have the "first common entry" treat
a null pointer as evidence that there's no printer as to have some other
elaborate scheme.
Allowing a NULL printer value not
only creates problems in the actual printer handling code but could cause
issues where the program has to decide what output to send to the printer
etc.

It's *at worst* no different at all from yours. There are many easily
constructed cases where it's better.
In my model, it is illegal to call print handling code if the printer is
not valid in the first place. This would be enforced by assertions to make
sure the code is never used in this way. Since the code is never even
called for an invalid printer, there is no way that any piece of code which
was not properly designed to handle the missing printer could cause any
issues. I call this more robust.

This is too vague and hand-wavey to constitute an argument or a design
description. Present a complete, self-contained, example program showing
how you think this would work. I'm not buying it at all; so far, it sounds
like you're inventing a huge amount of overhead in order to synchronize
two pieces of information, and thus introducing an entire new *class* of
errors that are impossible with the standard idiom.
The simplest thing to do isn't always the best thing to do.

Not always, but until you have an explanation of something better which
is both concrete and coherent, and actually shows some kind of evidence
for why it's better other than your dislike of the completely standard
C idiom, which everyone is already used to and using and which works reliably
in billions of lines of code, I'm going to do something simple and reliable.
Congratulations, this is the first decent argument that I have seen used in
this sub-thread. I do however disagree that this is a problem.
1. I would not even enounter this problem because I have made sure this
incident never arises as in my model, this code never would have
been called.

You're just pushing the problem out a layer of abstraction without changing
it.

SOME code is called. Doesn't matter whether it's this code or your
completely gratuitous and probably buggy abstraction layer that you invented
to work around the standard C idiom. Some code is called, and must deal
with the possibility that no printer is available.

I'm using your printer layer. I want to print something. I must deal with
the possibility that there is no printer available. It doesn't matter
whether I deal with this by checking whether a pointer is null or doing
something else; I still have to deal with it.
2. I don't think this really adds any significant complication and omits
the "special case" far earlier so that the same bit of code is not
complicated in having to use a value in two different ways.

If sentinel values were some kind of incredibly arcane feature that no one
had ever heard of and that an ordinary C programmer might never have
encountered, this would be an argument.

They're not. It isn't.

There is no way to get away from the existence of null pointers, and the
possibility that a given pointer might be null when it's handed to your
code. You have to write the code for that anyway. By long-standing idiom,
that's the way to indicate "and we don't have one of these".

My solution:
void error_out(char *msg) {
fprintf(stderr, "fatal error: %s\n", msg ? msg : "unknown error");
exit(1);
}
...
error_out("couldn't open a file, sorry.");
...
error_out(NULL);


Your solution:
typedef enum error_message_availability {
ERROR_NOT_AVAILABLE,
ERROR_AVAILABLE,
} error_message_availability_t;

void error_out(error_message_availability_t avail, char *msg) {
if (avail == ERROR_AVAILABLE)
fprintf(stderr, "fatal error: %s\n", msg);
else
fprintf(stderr, "fatal error: unknown error\n");
exit(1);
}
...
error_out(ERROR_AVAILABLE, "couldn't open a file, sorry.");
...
error_out(ERROR_NOT_AVAILABLE, /* what goes here? */);

In your system, it is now possible to write:
error_out(ERROR_AVAILABLE, NULL);
and cause a null pointer dereference. In mine, it's not.

Furthermore, you still haven't suggested what should go in the "msg" slot
if there is no error. Or do you want us to do:

void error_out_with_message(char *msg) {
fprintf(stderr, "fatal error: %s\n", msg);
exit(1);
}
void error_out_without_message(void) {
fprintf(stderr, "fatal error: unknown error\n");
exit(1);
}
...
if (error_availability == ERROR_AVAILABLE)
error_out_with_message("couldn't open a file, sorry.");
else
error_out_without_message();

This is just ridiculous. You still have to deal with the fact that someone
can pass a pointer to your code, and that pointer could be a null pointer.
You can never escape that in any program which uses dynamic allocation. If
you allow users to specify the thing they want you to operate on by passing
a pointer to it, the pointer can be null. If you don't, you have a crippled
interface and people won't be able to make much use of it.

-s
 
T

Tim Harig

I am not sure this is actually that much better a practice, especially in C,
where it's pretty much idiomatic to use a null pointer to indicate "we don't
have a foo".

Yes it is idomatic and common; but, in my experience it also contributes to
a number of bugs. I am not conserned how tradional something has become.
If it is creating bugs then, it is ineffective and the practice needs to be
replaced as much as possible to prevent said bugs.
And also forced the user to know how to allocate a printer, which does nothing
but push the issue somewhere else. Now the user has to write:

printer_dev *p = get_new_printer();
if (p)...

So you're right back where you started.

I am assuming that whatever code creates a printer is wrapped in a
function. It should be. Code that accesses a printer should not have to
know anything about the printer structure any more then the programmer
should know about FILEs.
You can, of course, do:
fulfillment create_printer(printer_dev **printer);
and declare that, if it doesn't return SUCCESS, it hasn't done anything to
"printer".

No, then you have the same problem and you are gaining the worst of BOTH
implementations.
You can never eliminate the possibility of a pointer being null-or-invalid
unless it's the address of a static object. For most real-world cases,
dynamic allocation is necessary. You can't detect an invalid pointer, so
you have to use null as the "we don't have one" state.

I did assume dynamic allocation. The only difference is that the printer
handling code no longer does its own allocation. The calling code
allocates memory for the print_dev structure and is responsible for
releasing it when it is done.
Since you MUST do that, it is ridiculous to then attach a second status that
isn't the pointer's status, because the moment you do that, you've created
two pieces of information which are supposed to reflect the same thing, and
thus the possibility for them to be out of sync.

No, you have created one status that should be used. The stucture should
be unallocated and it is a good idea to change the pointer to NULL so that
the program segfaults if it is dereferenced rather then invoke undefined
behavior.

Personally, I would have saved all of the available printers to some kind
of collection. I would not have used a pointer after the printers were
allocated. Then, I would simply pass the pointer to the data structure
holding the printers, which may be empty.
Right, because at the point where you'd be *using* the printer, you don't
care why. It's only at the point that an attempt was made to *allocate*
a printer that you care why, and the error handling for that belongs
there.

You do when you attempt to initialize the printer. Did the printer fail
because there was no printer, because we do not permissions to access the
printer, etc. All of these different variations on the failure can either
be handled or at least logged for troubleshooting purposes.
That would be "much, much, harder to arrange". You're creating bloat in
code, data storage, and layers of abstraction, all to avoid something that
is clear, idiomatic, and useful.

No, I gave my opinion of the code design because I also questioned it. If
you don't want to accept these suggestions you don't have to. It was not
part of the argument against ?:. This is why I had to split my comments
below.

It is not in fact much, much harder to arrange in real code. Yes, it
entails more then was given in the code fragment; but, in the real world
there would be error handling infrastructure to deal with these kinds of
problems, and others, already in place. This just uses what should already
be there.
This seems very speculative, and you've repeatedly talked about adding
additional layers of interfaces, which sounds like a lot of overhead to
satisfy a prejudice.

In C, a null pointer is idiomatically used to indicate the unavailability
of a thing that would otherwise have been pointed to. That's the idiom
the language standard uses, for everything from malloc() to fopen(). Using
any other idiom is gratuitously confusing.

1. This would have been my recommendation up and above whether or not to
use the ?:.

2. Yes, the NULL pointer is idiomatic and tradional; but, also is a common
source of bugs. I don't care about being traditional. I care
about creating bugless code.

3. This is one of the coding standards that we use in house. It has been
largely effective. The decision only comes in to play when
creating the function interfaces. We have not had any problems
with people understanding how to use them. We have less problems
then we had before we added this to the coding standards.
I don't buy this notion of a "first common entry into the printing system".
The printing system is used by the rest of the program; every part of the
rest of the program that might wish to print has to know how to tell what
it can or can't do.

Given the example of a traditional GUI text editor. In my code, when the
app starts, it would initialize the printing system. If there are no
printers available, no option to print would be available. The only places
that need to test whether the printing system is properly established is
the dialog code that adds the ability to print to the menu and possibly the
wrapper function that is called when the user generates a keystroke
designated to print. If the printing system is not initialized, then the
menu item is not displayed and the function that wraps the keystroke
effectively becomes a noop.

Now, with the test in just two places, the system will never even allow the
possiblity of calling the printing code unless the printing system is
properly initialized.
Not really. It's just as practical to have the "first common entry" treat
a null pointer as evidence that there's no printer as to have some other
elaborate scheme.

You could also use the NULL pointer. The operation is equivilant; as long as
it is done in the dialog code that allows printing and the keystroke
wrapper function the printer handling code need never be accessed.
It's *at worst* no different at all from yours. There are many easily
constructed cases where it's better.

Again, use NULL if you prefer that. We have found it to cause more issues
then separating state from data. This is all really irrelevant to my
points on ?:
This is too vague and hand-wavey to constitute an argument or a design
description. Present a complete, self-contained, example program showing
how you think this would work. I'm not buying it at all; so far, it sounds
like you're inventing a huge amount of overhead in order to synchronize
two pieces of information, and thus introducing an entire new *class* of
errors that are impossible with the standard idiom.

I already explained that the tests were in the dialog generation code and
the keystroke wrapper code. For all of the printer handling functions it
would be defined as illegal to pass them a NULL pointer for the printer
field. The very first lines in all of our functions are assert statements.
One of those asserts would be:

assert(printer != NULL);

Now, if any of these functions receive a NULL printer it is because there
is an error in the calling code. During testing, any errant calling code
that does pass a NULL pointer will cause the system to abort making it
clear that there is an error. Furthermore, it is easy to create unit tests
that deliberately return an unsuccessful printer intialization to see if
the code handles it correctly or if it passes the NULL on to the print
handling code which will abort to indicate something is wrong with that
unit of code.
Not always, but until you have an explanation of something better which
is both concrete and coherent, and actually shows some kind of evidence
for why it's better other than your dislike of the completely standard
C idiom, which everyone is already used to and using and which works reliably
in billions of lines of code, I'm going to do something simple and reliable.

Please differentiate my suggestions from my arguments against ?:. As
suggestions I have stated:

1. Any problems properly allocating a printer should be handled as soon as
possible. The farther an invalid printer is propogated throught
the code, the more opportunities that somewhere it will not be
handled throughly.

2. To achieve 1, I recommend not calling the printer code if there is no
valid printer to be used. This can be prevented by placing
restrictions in the GUI menu and keystroke wrappers that call the
printer infrastructure.

3. Moving the error conditions outside of the communication channel used
for transfering data reduces the possiblity that the user will
forget to properly check the validity of the data received.

4. That by separating the data and error status information, you have more
flexibility in handling specific errors that may occur.

All of these (except 2 depends on 1), are independant suggestions that may
be evaluated individually. None of them have a direct bearing on the ?:
argument.
You're just pushing the problem out a layer of abstraction without changing
it.

Yes, but by doing so, far less of the code is in a position to mishandle an
error condition.
SOME code is called. Doesn't matter whether it's this code or your
completely gratuitous and probably buggy abstraction layer that you invented
to work around the standard C idiom. Some code is called, and must deal
with the possibility that no printer is available.

I didn't do this the remove the C idiom. I moved the error handling to the
top where there is less exposure in the code. If you prefer, this can
still be done using NULLs and ?:s. This is a completely separate point.
I'm using your printer layer. I want to print something. I must deal with
the possibility that there is no printer available. It doesn't matter
whether I deal with this by checking whether a pointer is null or doing
something else; I still have to deal with it.

You may want to print something; but, you cannot because there is no
printer available. You don't need to deal with it, because it has already
been delt with far higher up in the codebase. There is no way of getting
to where you are that you want to print something. The option to do so has
been removed.
If sentinel values were some kind of incredibly arcane feature that no one
had ever heard of and that an ordinary C programmer might never have
encountered, this would be an argument.

They're not. It isn't.

The point is that they are not necessary. The do not provide anything that
cannot be done using an if/else.

We added not using ?: to our coding standards because it created bugs that
were clearer to see when they had been converted to if/else syntax using
the exact same logic. I don't disagree that they can be used effectively.
What I disagree with is that too many people who think that they are
using them effectively are not by the fact that they have used them to
obfuscate bugs in their code.

Furthermore, since ?: is usually used on a single line, it violates the
expectation that control statements will occupy multiple commonly indented
lines. This means that it is easy to overlook this conditional when
scanning a piece of code looking for bugs.

If ?: actually provided something that if/else could not. We do allow goto
and, because of the stigma that most programmers have inhereted for it, we
see it abused far less. As it is, it does not. It is just a measure
to decrease typing. We do not see the cost of typing overwaying the cost
of adding bugs into our codebase.
There is no way to get away from the existence of null pointers, and the
possibility that a given pointer might be null when it's handed to your
code. You have to write the code for that anyway. By long-standing idiom,
that's the way to indicate "and we don't have one of these".

Yes, NULL pointers exist. If they are handled improperly, they cause bugs.
The longer they are allowed to permiate the code base, the longer that they
will be mishandled somewhere. Therefore, we have found it prudent to
handle them as soon as possible; even if it violates the long standing
idiom. Long standing idioms to not have to account for the bugs that I
create using them. I do.
My solution:
void error_out(char *msg) {
fprintf(stderr, "fatal error: %s\n", msg ? msg : "unknown error");
exit(1);
}
...
error_out("couldn't open a file, sorry.");
...
error_out(NULL);


Your solution:
typedef enum error_message_availability {
ERROR_NOT_AVAILABLE,
ERROR_AVAILABLE,
} error_message_availability_t;

void error_out(error_message_availability_t avail, char *msg) {
if (avail == ERROR_AVAILABLE)
fprintf(stderr, "fatal error: %s\n", msg);
else
fprintf(stderr, "fatal error: unknown error\n");
exit(1);
}
...
error_out(ERROR_AVAILABLE, "couldn't open a file, sorry.");
...
error_out(ERROR_NOT_AVAILABLE, /* what goes here? */);

That would *NOT* be my solution. My solution would never call a function
outputing a message unless the message was available to begin with. I
would call a different function. Any inputs would be required to be valid
and they would be asserted to be so.

It is my philosophy that errors should be handled where they are found
rather then being pushed off to another function hoping that it can
handle the error cleanly. If you send data to any function; then,
you should have sanized it to make sure that it was valid.

When functions encounter errors, they should cleanly indicate that an error
has occured and if possible why.
This is just ridiculous. You still have to deal with the fact that someone
can pass a pointer to your code, and that pointer could be a null pointer.

Functions have specifications. If the specification of a function says
that the function should not be given a NULL pointer, then the programmer
who called the function with a NULL pointer is at fault. The function
should assert this and abort should it ever be passed a NULL pointer.
You can never escape that in any program which uses dynamic allocation. If

I don't, I am assuming dynamic allocation.
you allow users to specify the thing they want you to operate on by passing
a pointer to it, the pointer can be null. If you don't, you have a crippled
interface and people won't be able to make much use of it.

Our inteface makes use of this principle. When programmers violate it, the
asserts cause the code to abort making it *very* clear that they have done
something wrong. We write code that is designed to be *highly* stable and
we add far more infrastructure then most projects do in the process. The
coding guidlines that we have adopted help us to do that. NULL pointers
are a common source of issues; so, we have found ways of working around
them so that they are avoided as much as possible and if they are not
properly sanitized, we have taken offorts to assure that they are more
likely to crash during testing rather then escaping into shipping code.
 
P

Phil Carmody

Keith Thompson said:
What Tim meant, as he clarified in a later followup, is that the
use of ?: implies, or should imply, that the resulting value is
going to be used, i.e., not discarded. I tend to agree; if you're
going to discard the result, you might as well use an if statement.

Agreed. So much agreed that it's a policy I work to. If you can
sniff out our core-review mailing list (ok.r.n.c) you'll see me
rejecting a ?: with an unused evaluation less than a week ago.
(Describing it as 'macho code' more suited to the IOCCC than the
linux kernel.)

/However/, if you are going to use the result, and you're the only
user of the test, then you might as well use the ? : operator, surely?

So they're both useful, for their own particular cases or niches?

If that's "yes, yes", then we're in complete agreement, and in
clear disagreement with TH.
Obviously "assignment" wasn't the righ word for this, but one can
very loosely think of the result of an expression being "assigned"
to the enclosing context.

No wording would have made what he said correct in my eyes.
Why use both '+' and '-'? '-', in sufficient quantities, can
do everything that '+' can do! It's just bad logic.

Phil
 
S

Seebs

Yes it is idomatic and common; but, in my experience it also contributes to
a number of bugs.

I am not convinced of this. Furthrermore, avoiding the standard way of doing
something creates a lot of bugs too.
I am not conserned how tradional something has become.
If it is creating bugs then, it is ineffective and the practice needs to be
replaced as much as possible to prevent said bugs.

Nothing you've posted here has looked to me as though it could POSSIBLY
result in fewer bugs.
I am assuming that whatever code creates a printer is wrapped in a
function. It should be. Code that accesses a printer should not have to
know anything about the printer structure any more then the programmer
should know about FILEs.

Very good!

But "whether or not I have a printer" is not anything about the printer
structure.

Let's get more specific. In your implementation, do I ever see a
(printer_dev *)? If so, what is its value if I don't have a printer?

If your solution is "never call any code that could possibly have used
that structure unless there is a printer", we have interesting issues. How
do I decide whether or not to call the code that could have used that?
If I do want to use it, what do I do to tell that code what printer it
gets? If I pass a pointer to it, what happens if the pointer is a
null pointer? (And no matter what you think about planning, it WILL be
a null pointer sooner or later, because That Is What Happens. Code
defensively!)
No, then you have the same problem and you are gaining the worst of BOTH
implementations.

Why, yes. But you haven't given me any way to get this thing that's like
a pointer but is never null yet.
I did assume dynamic allocation. The only difference is that the printer
handling code no longer does its own allocation. The calling code
allocates memory for the print_dev structure and is responsible for
releasing it when it is done.

If I might quote something:
Code that accesses a printer should not have to
know anything about the printer structure any more then the programmer
should know about FILEs.

Remember that?

You are now requiring the CALLING CODE to do the allocation and release.
Meaning the calling code now needs to know what size a print_dev structure
is, and allocate it, and maintain it. That's EXACTLY what you just said
the calling code shouldn't have to do.
No, you have created one status that should be used. The stucture should
be unallocated and it is a good idea to change the pointer to NULL so that
the program segfaults if it is dereferenced rather then invoke undefined
behavior.

Uh.

First off, dereferecing null pointers *IS* undefined behavior -- and it is
*not* guaranteed to segfault. Secondly, you're missing my point.

If your status *is* the pointer, then it is impossible for you to think you
have a pointer when it's null.

If your status is *anything other than* the pointer, then it is possible for
the status to indicate that you have a printer, but the pointer is a null
pointer.

You've created a new point of failure.
Personally, I would have saved all of the available printers to some kind
of collection. I would not have used a pointer after the printers were
allocated. Then, I would simply pass the pointer to the data structure
holding the printers, which may be empty.

And what happens if that allocation fails? We're right back to "you have
to check whether the pointer is null".
You do when you attempt to initialize the printer. Did the printer fail
because there was no printer, because we do not permissions to access the
printer, etc.

There are two possibilities:

1. These errors occurred during the attempt to create a pointer-to-printer,
and were logged/diagnosed at that time. We have already logged or diagnosed
that error, and the fact that we are trying to access an unavailable printer
is now a separate error which has nothing to do with why there is no
printer available.

2. These errors will be generated while interacting with a valid
pointer-to-printer which is not a null pointer.
2. Yes, the NULL pointer is idiomatic and tradional; but, also is a common
source of bugs. I don't care about being traditional. I care
about creating bugless code.

Which is why you've proposed a system which has several more possible failure
modes and requires more layers of allocation and bookkeeping, dramatically
increasing the probability of bugs.
3. This is one of the coding standards that we use in house. It has been
largely effective. The decision only comes in to play when
creating the function interfaces. We have not had any problems
with people understanding how to use them. We have less problems
then we had before we added this to the coding standards.

This does not show that the particular standard you picked is a good one,
only that paying attention to an issue reduces bugs involving it.
Given the example of a traditional GUI text editor. In my code, when the
app starts, it would initialize the printing system. If there are no
printers available, no option to print would be available. The only places
that need to test whether the printing system is properly established is
the dialog code that adds the ability to print to the menu and possibly the
wrapper function that is called when the user generates a keystroke
designated to print. If the printing system is not initialized, then the
menu item is not displayed and the function that wraps the keystroke
effectively becomes a noop.

Now, with the test in just two places, the system will never even allow the
possiblity of calling the printing code unless the printing system is
properly initialized.

Yes. I wrote a system very much like this, and in the system very much like
this I wrote, I do roughly that, only I use the null/non-null pointer to
determine whether a printer was found, and it works fine.
You could also use the NULL pointer. The operation is equivilant; as long as
it is done in the dialog code that allows printing and the keystroke
wrapper function the printer handling code need never be accessed.

In other words, none of this has ever been an argument against using null
pointers as a sentinel to determine that no printer is available?
Please differentiate my suggestions from my arguments against ?:. As
suggestions I have stated:
Okay.

1. Any problems properly allocating a printer should be handled as soon as
possible. The farther an invalid printer is propogated throught
the code, the more opportunities that somewhere it will not be
handled throughly.

Yup, agreed.
2. To achieve 1, I recommend not calling the printer code if there is no
valid printer to be used. This can be prevented by placing
restrictions in the GUI menu and keystroke wrappers that call the
printer infrastructure.

Makes sense.
3. Moving the error conditions outside of the communication channel used
for transfering data reduces the possiblity that the user will
forget to properly check the validity of the data received.

But increases the possibility that the check will be handled incorrectly
resulting in a garbage or invalid pointer being used, which is a lot harder
to catch usually than a null pointer is.
4. That by separating the data and error status information, you have more
flexibility in handling specific errors that may occur.

This, I haven't seen, though. By the point at which you get back the null
pointer, the error should ALREADY have been handled or diagnosed.
All of these (except 2 depends on 1), are independant suggestions that may
be evaluated individually. None of them have a direct bearing on the ?:
argument.

Fair enough.
You may want to print something; but, you cannot because there is no
printer available. You don't need to deal with it, because it has already
been delt with far higher up in the codebase. There is no way of getting
to where you are that you want to print something. The option to do so has
been removed.

That doesn't make any sense. I'm writing a hunk of code. I think it'd be
fun to, when this code is hit, print something. I can call the printer
code at this point.

In short, removing the menu item doesn't remove the API. Yes, you can tell
people never to call the printer code unless the printer driver has been
initialized, but you can tell them that in any environment.
We added not using ?: to our coding standards because it created bugs that
were clearer to see when they had been converted to if/else syntax using
the exact same logic.

This, I'd probably mostly agree with, but I'm not sure I'd always agree
with it.

There are a whole lot of things of the form:
printf("%s", x ? x : "<nil>");
which lose a lot of readability if you don't use ?:.

There are indeed a lot of places where if/else or a switch is clearer,
though.
Yes, NULL pointers exist. If they are handled improperly, they cause bugs.
The longer they are allowed to permiate the code base, the longer that they
will be mishandled somewhere. Therefore, we have found it prudent to
handle them as soon as possible; even if it violates the long standing
idiom. Long standing idioms to not have to account for the bugs that I
create using them. I do.

All you're doing is replacing the risk of null pointers with the risk of null
pointers amplified by the risk that your other piece of data which is supposed
to tell you whether or not the pointer is valid will be wrong.
That would *NOT* be my solution. My solution would never call a function
outputing a message unless the message was available to begin with. I
would call a different function.

This seems like it creates a lot of code duplication in likely error
handlers. I'd rather have a single error handler which may or may not
receive a given piece of data, and reacts accordingly, than several different
error handlers each of which takes different inputs and tries to do otherwise
the same things.

Imagine that there are several paths which can lead to needing to
clean up a bunch of data structures. In some of them, there is a useful
diagnostic message to provide; in others, there is not. You seem to be
advocating duplicating the entire cleanup thing to avoid having something
to the effect of:

if (msg) {
fprintf(stderr, "message: %s\n", msg);
}

in the middle of a function.
Any inputs would be required to be valid
and they would be asserted to be so.

So we duplicate all functions for every possible combination of available
inputs.
Functions have specifications. If the specification of a function says
that the function should not be given a NULL pointer, then the programmer
who called the function with a NULL pointer is at fault. The function
should assert this and abort should it ever be passed a NULL pointer.

Yes, because nothing makes users happier than programs which coredump
without even a moment's consideration given to saving files or trying to
recover gracefully.
Our inteface makes use of this principle. When programmers violate it, the
asserts cause the code to abort making it *very* clear that they have done
something wrong. We write code that is designed to be *highly* stable and
we add far more infrastructure then most projects do in the process. The
coding guidlines that we have adopted help us to do that. NULL pointers
are a common source of issues; so, we have found ways of working around
them so that they are avoided as much as possible and if they are not
properly sanitized, we have taken offorts to assure that they are more
likely to crash during testing rather then escaping into shipping code.

That overall sounds like it makes sense, but the specific examples you've
given have seemed like they'd introduce a large number of points of failure,
all different.

This seems like a case where, in order to absolutely eliminate a hundred bugs
which were all similar, you've introduced several hundred bugs all different
and hard to group together.

I'm all for programs asserting that inputs are not null if it doesn't make
sense for them to get called without an input, but there are a whole lot of
cases where an input could reasonably be optional, at which point, a pointer
which may be null is an extremely clear way to handle that.

-s
 
P

Phil Carmody

Tim Harig said:
1. I have already said that I don't mind as much when used in an output
statement because it doesn't alter the control flow structure of
the program.

If you have said that, you've not said it as clearly as that before,
and in fact quite the opposite, you've said some very mangled and
unclear things before while addressing whether ?: should be used.
2. Your example is non-sequator to the argumentation that followed it.
Hilarious.

3. It would seem obvious to me to either change the function declaration so
that it uses the same values as p->is_big or modify the struct
definition so that it uses the same values expected by the
function. Personally, p->is_big should have been an enumeration
in the first place.

It should be a boolean, and probably was in the absense of evidence
to the contrary.
This isn't a ?: problem, it is a design problem.

Would you say that if I s/(BIG|SMALL)/$1_BUFFER_SIZE/g ?
If you have so little imagination that you can't imagine those two
tokens representing sizes, then you're very lacking in imagination.

Phil
 
T

Tim Harig

If you have said that, you've not said it as clearly as that before,
and in fact quite the opposite, you've said some very mangled and
unclear things before while addressing whether ?: should be used.

In <[email protected]>:

-> printf("%d %s in %d
-> Director%s\n",nfiles,(nfiles=1?"File":"Files"),ndirs,(ndirs=1?"y":"ies"));
-
-Fair enough, this is a different usage then above because you are not
-actually going to be changing any persistant values which may be used later
-in the program. I would accept this; although, I am assuming that you have
-already check for 0 and negative values elsewhere in your code.

I accept that is a valid use. That doesn't mean that I consider it
significant enough that I am willing to issue the statement that I condone
the use of ?:. It is still just as clear using an if/else, it just saves a
little typing. I am not arguing that ?: is not sometimes handy. The
problem, as it used to be with goto, is that people are not always good at
knowing when is appropriate to use and inevitably somebody abuses it.

Many coding standards now prohibit the use of goto. They are justified
because of the problems that it has caused in spite of its benefits. I
personally don't mind goto. goto, even though it is sometimes abused,
at least provides functionality that is not otherwise available. ?: doesn't
really provide anything, other then a little less typing, that if/else does
not.
It should be a boolean, and probably was in the absense of evidence
to the contrary.

If it is then why not just use it? You are then adding a construct that
didn't need to be there.
Would you say that if I s/(BIG|SMALL)/$1_BUFFER_SIZE/g ?
If you have so little imagination that you can't imagine those two
tokens representing sizes, then you're very lacking in imagination.

1. Then you should have made your names more explicit.

2. There still is no necessity for a ?:
 
S

Seebs

Many coding standards now prohibit the use of goto. They are justified
because of the problems that it has caused in spite of its benefits. I
personally don't mind goto. goto, even though it is sometimes abused,
at least provides functionality that is not otherwise available. ?: doesn't
really provide anything, other then a little less typing, that if/else does
not.

If "a little less typing" means "a large and reasonably complicated
expression not being duplicated", that can be a very big thing.

My rule is this:

If I feel there's a genuine control flow difference involved, in that the
things done are different, I use if/else.

If I feel that there is only a choice of data involved, and the things done
would be the same in both cases, I tend to use ?: instead.

Used that way, it adds clarity to the code, by making it clear to the user
that there isn't a significant shift in functionality, only a value which
may vary under differing circumstances.

-s
 
T

Tim Harig

I am not convinced of this. Furthrermore, avoiding the standard way of doing
something creates a lot of bugs too.

Obviously, your spatzing out over a single design decision that is
irrelevan to my main point anyway and which is a non-issue in practice.
I am assuming that whatever code creates a printer is wrapped in a
function. It should be. Code that accesses a printer should not have to
know anything about the printer structure any more then the programmer
should know about FILEs.
[SNIP]
But "whether or not I have a printer" is not anything about the printer
structure.

1. I already stated that I would personally collect the printers in
collection. There would be no reason to make a pointer to any
given printer unless you were going to actually use that printer.
Since the printer code is not called when there are not any
printers, this pointer would never be created.

2. The same thing applies even if we create the pointer ahead time. Since
the print_dev structure is used as a black box, nothing should be
using if except for the printer handling system. That isn't even
accessed unless there is actually a printer present.
Let's get more specific. In your implementation, do I ever see a
(printer_dev *)? If so, what is its value if I don't have a printer?

See 1. If it is statically created in a major scope then it should not
matter; but, I would advocate making it NULL as there is a higher chance
that it will crash the system during debugging if it used.
If your solution is "never call any code that could possibly have used
that structure unless there is a printer", we have interesting issues. How
do I decide whether or not to call the code that could have used that?

Since the printer status is separate from the printer_dev data, you would
use the flag returned from the function that generates the printer_dev
structure if we are dealing with a single device. If you use my idea of
using a collection, then you would simply check the length of the
collection to make sure that is not empty. In this case, there was never
any pointer explicitly defined to have to worry about.
If I do want to use it, what do I do to tell that code what printer it
gets? If I pass a pointer to it, what happens if the pointer is a
null pointer? (And no matter what you think about planning, it WILL be
a null pointer sooner or later, because That Is What Happens. Code
defensively!)

If somebody chooses to violate this design decision and tries to pass a
untested pointer to one of the functions that does now allow NULL as part of
its specifications, then they will quickly generate an abort from the
functions assertion during the unit tests that are triggered before they
can check in the code.
Why, yes. But you haven't given me any way to get this thing that's like
a pointer but is never null yet.

My original function was:

fulfillment create_printer(printer_dev* printer);

It does not create a printer, it only writes to the location of the printer
that it is given. It also asserts on receiving a NULL pointer as the user
should have check the return from malloc(). If it returns successful, then
you can, if you like, use the printing infrastructure. If it fails, then
for some reason a proper printer_dev could not be created. The one that
was previously allocated would likely be freed as it is not needed. If the
pointer itself is statically allocated then it should never be used. You
may set it to NULL if you like.
If I might quote something:


Remember that?

You are now requiring the CALLING CODE to do the allocation and release.
Meaning the calling code now needs to know what size a print_dev structure
is, and allocate it, and maintain it. That's EXACTLY what you just said
the calling code shouldn't have to do.

Just because it is a black box to the programmer does not mean that it is a
black box to the compiler. The compiler still knows what size a
struct printer_dev is an will execute sizeof appropriately. This is not an
issue. Obviously, the code must keep track of the printer_dev so that it
can be passed to the proper functions.

The alternative here, if you are using my printer collection method, would
be to make the printer collection global so that it doesn't need to be
explicity passed between functions. (See how curses uses stdscr as a
default window for many functions) I don't have any major issues against
this.
First off, dereferecing null pointers *IS* undefined behavior -- and it is
*not* guaranteed to segfault. Secondly, you're missing my point.

This is true; but, NULL is still more likely to create a crash as any other
value. Since all of the functions that should be accessing it abort on
NULL values, any attempts to pass this pointer after a printer initization
failure cause an abort in the unit tests making immediately clear that
there is a problem.
You've created a new point of failure.

No, as long as the pointer existed, the point of failure was there.

1. I have limited the opportunity for that failure by preventing any code
that should be accessing this pointer from running whether or not
it is actually able to process the NULL pointer.

2. I have added assertions so that if some code does try to use the printer
handeling code it is quickly identified during the unit tests.
And what happens if that allocation fails? We're right back to "you have
to check whether the pointer is null".

No, then the printer is never added to the collection and the collection is
simply empty.
There are two possibilities:

1. These errors occurred during the attempt to create a pointer-to-printer,
and were logged/diagnosed at that time. We have already logged or diagnosed
that error, and the fact that we are trying to access an unavailable printer
is now a separate error which has nothing to do with why there is no
printer available.

The errors I indicated are relevant when we try to create the printer
object. Different errors may need to be handle differently or at least to
indicate the different reasons why the printer is not available to the
user.

After that, no printer code should be running if we cannot intialize at
least one printer.
2. These errors will be generated while interacting with a valid
pointer-to-printer which is not a null pointer.

Errors involving a real printer are different those pertaining to
initializing the printer driver object. The printer may be out of paper;
but, it does exist.
Which is why you've proposed a system which has several more possible failure
modes and requires more layers of allocation and bookkeeping, dramatically
increasing the probability of bugs.

I really have done very little bookkeeping. I have added tests in two
places that prevent the entire printing system from being accessed if it
is not relavant to do so. I have prevented this entire section of code
from being able to cause errors in the event that no printer exists. I
have added assertions to my functions to make sure it is apparent should
they be used improperly. The differences are really minor. All I have
really done is limited the amount of code that could be causing problems.

1. Less code to cause problems == less bugs.
2. More assertion statements means unit tests catch more bugs faster.
3. More information about why the printer is unavailable means more
troubleshooting information to the user.
4. Untangling the status information from the information makes testing for
error conditions more explicit and less error prone.
This does not show that the particular standard you picked is a good one,
only that paying attention to an issue reduces bugs involving it.

Since it reduces bugs then it has served its purpose.
Yes. I wrote a system very much like this, and in the system very much like
this I wrote, I do roughly that, only I use the null/non-null pointer to
determine whether a printer was found, and it works fine.

Yes, you seem to be stuck on that issue. All of my other suggestions will work
by testing the null pointer. We do so because we have found, that do to
its explicit nature, it has a tendency to prevent errors that arise from
improperly sanitizing the mixed data/status stream not because it allows
any of my other suggestions.
In other words, none of this has ever been an argument against using null
pointers as a sentinel to determine that no printer is available?

I have made several independent suggestions.

One of them was to split the channels used for passing information and
those used for status. This includes things like requiring a signed type
so that -1 can be passed as an error condition, using wider fields so
that error or formating information can be passed along with character
information, or passing NULL as an error condition.

My printer allocation function makes it very explicit when an error occurs
without intermingling the information with the data potentially returned.
It also makes it easy to send more information about what actually happened
as opposed to just success or failure by adding new error conditions to the
enumeration returned.

Not doing these things doesn't prevent you from using my other suggestion
to prevent the calling of printer handling code if there is no printer.
That can be done using NULL flagging if you wish. We do this because we
have found that it decreases bugs; but, these are two independent
suggestions. You can certainly use one without the other.
But increases the possibility that the check will be handled incorrectly
resulting in a garbage or invalid pointer being used, which is a lot harder
to catch usually than a null pointer is.

Actually, it makes the check explicit rather then requiring that the data
be checked. Since our decoupling *never* creates a pointer (it uses the
pointer that it was given) and does not require the use of a pointer,
any invalid pointer problems must a problem with the calling code.

The return value of the function is the status of the function itself.
It doesn't require any interpretation to know whether the error conditions
could be null or -1 or BLACK_BOX_CONSTANT as are common with so many
function designs. If the function doesn't return SUCCESS, then it had an
error and the data from it should not be used. I cannot force everybody to
check return values; but, if they do, there is not mistaking what is an
error and what is not. Here, every non-void returning function must be
checked. We don't seem to have any problems doing this correctly.

Note that I have recommended changing unused pointers to NULL to help find
any pointers that should not be used but which were.
This, I haven't seen, though. By the point at which you get back the null
pointer, the error should ALREADY have been handled or diagnosed.

If you have a function like:

printer_dev* create_printer(void);

If you receive a NULL how do you know whether you did not have sufficient
permissions to access the device or whether there was not enough memory to
allocate the printer_dev object? Both error conditions should be handled
separately. Presumably you resort to setting a global variable with
details on error condtions? (ie, errno). We have found that this can be
troublesome. For us, it is much easier to pass the information back
directly:

typedef enum error_state_type {
SUCCESS,
MEM_ERROR,
PERM_ERROR,
PDNE_ERROR //printer does not exist
} error_state;

error_state create_printer(printer_dev* new_priner);
That doesn't make any sense. I'm writing a hunk of code. I think it'd be
fun to, when this code is hit, print something. I can call the printer
code at this point.

If your code attempts to call the printer, it will hit the assertion and
abort soon after you submit it to the unit tests.
In short, removing the menu item doesn't remove the API. Yes, you can tell
people never to call the printer code unless the printer driver has been
initialized, but you can tell them that in any environment.

But again, with the assertion statements, attempting to do so will case it
to fail the unit tests.
This, I'd probably mostly agree with, but I'm not sure I'd always agree
with it.

If ?: provided any real functionality then there would be an argument to
allow it under out standards. As it is, it adds nothing of real value,
while potentially causing problems. We prefer to err on the safe side.
There are a whole lot of things of the form:
printf("%s", x ? x : "<nil>");
which lose a lot of readability if you don't use ?:.

They don't really loose reability, they just become a little longer:

/* print x if available or print "<nil>" as a substitute */
if (x == NULL)
printf("%s", x);
else
printf("%s", "nil");

Note that I don't make any assumptions about C's true or falsehood values.
The code explicitly states what it is testing for. This is much clearer
for those who may be coming from other languages without the same values.

Note that I have added a comment explaining exactly why a conditional
output was being used. This is something I rarely see done with ?:.
All you're doing is replacing the risk of null pointers with the risk of null
pointers amplified by the risk that your other piece of data which is supposed
to tell you whether or not the pointer is valid will be wrong.

Yep, and our bug reports suggest a distinct drop in errors caused by
pointers that were improperly checked against being NULL and other errors
where return values were not fully checked for error conditions.
This seems like it creates a lot of code duplication in likely error
handlers. I'd rather have a single error handler which may or may not
receive a given piece of data, and reacts accordingly, than several different
error handlers each of which takes different inputs and tries to do otherwise
the same things.

We have two very in-depth error handlers. One for normal situations and
one that is safe to use for memory conditions because it does not attempt
to allocate more memory. It has the ability to trace strings of errors, it
has canned reactions for most error conditons, it has its own
internationalized canned routines for handling most common error condtions.
Do to our high stability requirements, it has the ability to send important
data and restart requests back to a program which monitors the main process
for errors.

Our system works quite well with a minimum of code duplication. For
most errors, we need only send it the system error code and it will trace,
log, send error messages as appropriate. It has other fuction interfaces
that can be used for abnormal situations. These also have specifications
about how they can be used.

I evaluated your "error handling" function as I would for any other function.
Imagine that there are several paths which can lead to needing to
clean up a bunch of data structures. In some of them, there is a useful
diagnostic message to provide; in others, there is not. You seem to be
advocating duplicating the entire cleanup thing to avoid having something
to the effect of:

See above, our error system handles most of this automatically. Note that
you could also just enter the extra comments field as a blank string "\0"
if you didn't have anything to add.
So we duplicate all functions for every possible combination of available
inputs.

No, a function has its specification. If the input is invalid it is simply
invalid. There is not auxillery function to handle your invalid inputs
that you have not sanitized.
Yes, because nothing makes users happier than programs which coredump
without even a moment's consideration given to saving files or trying to
recover gracefully.

If you have assertions crashing in production code then you are using your
assertions improperly. Assertions basically allow you to have two pieces
of code through the use of the preprocessor. One is for production
(compiled with NDEBUG defined) and the other contains all of the assertion
tests. All of our functions validate as much of their input as possible
through assertions. If somebody passes a NULL value where it has been
forbidden the code will abort making it very clear that there is a problem.

This is a very powerful tool when combined with automated tests. For this
example, I would definitly have a test where no printer can be allocated.
These kinds of error conditions are hightly tested. If somebodies code
attemped to use pass an unsanized NULL as a printer, it would trigger an
assertion, and the test would fail.

The automated tests are triggered anytime somebody attempts to commit code
into the revision database. If it does not pass the tests, it cannot be
committed. By making the functions as restrictive as possible, we catch
many more errors automatically before we ever need to debug the code.
That overall sounds like it makes sense, but the specific examples you've
given have seemed like they'd introduce a large number of points of failure,
all different.

This seems like a case where, in order to absolutely eliminate a hundred bugs
which were all similar, you've introduced several hundred bugs all different
and hard to group together.

The trick isn't that we might not have added more bugs; but, that we have
made it easier to find them when we do.
I'm all for programs asserting that inputs are not null if it doesn't make
sense for them to get called without an input, but there are a whole lot of
cases where an input could reasonably be optional, at which point, a pointer
which may be null is an extremely clear way to handle that.

Again, we make our function interfaces as restrictive as possible. The
more restrictive the interface, the more tests that we can use to validate
that the input cannot contain errors. The point is not to be flexible
and easy for the programmer but to instill dicsipline and make it easier
to find bugs.
 
T

Tim Harig

My rule is this:

If I feel there's a genuine control flow difference involved, in that the
things done are different, I use if/else.

If I feel that there is only a choice of data involved, and the things done
would be the same in both cases, I tend to use ?: instead.

I don't really disagree with you here. It is a good rule.
Used that way, it adds clarity to the code, by making it clear to the user
that there isn't a significant shift in functionality, only a value which
may vary under differing circumstances.

You have made most of the best arguments in this thread. I can see that
you are a good programmer and if you were the only person that I had to
worry about; then, I have no problem with you using ?: in accordance to
your rule. Unfortunately, I have to deal with people who may not have your
disgression.
 
P

Phil Carmody

Tim Harig said:
2. There still is no necessity for a ?:

Ah, the old "no need" argument - I think even my mum gave up trying
to use that one several decades ago. There's no necessity for any
programming. There's no necessity for you on this newsgroup either.

Phil
 
P

Phil Carmody

Tim Harig said:
They don't really loose reability, they just become a little longer:

/* print x if available or print "<nil>" as a substitute */
if (x == NULL)
printf("%s", x);
else
printf("%s", "nil");

That's an exponential increase in the amount of code. Try the
same technique with:

printf("%s: %s([%s, %s])=%s, new state=%s\n",
state?state:"<no-state>",
op?op:"<no-op>",
v1?v1:"<no-v1>",
v2?v2:"<no-v2>",
res?res:"<no-result>",
newstate?newstate:"<no-state>");

Which benefits even more from gcc's two-operand '?:'.

Phil
 
N

Nick Keighley

What Tim meant, as he clarified in a later followup, is that the
use of ?: implies, or should imply, that the resulting value is
going to be used, i.e., not discarded.  I tend to agree; if you're
going to discard the result, you might as well use an if statement.
Obviously "assignment" wasn't the righ word for this, but one can
very loosely think of the result of an expression being "assigned"
to the enclosing context.

ah,ok. That makes sense. The ?: operator yeilds a value. If TH is
saying that ?: usage where the value is ignored is odd then I'll
agree. If he's saying all uses of ?: should be depracated and replaced
with if-else statments then I disagree. TH seems to be using a coding
a style that bans or heavily discourages use of ?:. I think that's a
bad idea.
 
N

Nick Keighley

No, I didn't have a more exact term to describe it, and since C does not
allow overloading, I thought that it might be descriptive in this case.

I've heard it termed "in band" as in "in band signalling". But I'm
from a comms background. Some protocols emded control information in
the datastream. It can be tricky to get right but handy when done
right. All the various escape sequences like \n and \a can be thought
of as a similar sort of thing. As is \0 in C strings.
The fact is that you are using p in two different ways to send two
different kinds of information.  This isn't really all that different from
the ways many standard functions like fgetc() return an int for what is
expected to be a char value so that they can also send error information
along with any values.  This is questionable practice that tends to cause
lots of trouble (in this case particularly to newcomers.)

yes it's the same sort of thing and can cause problems. I used to work
with someone who banned in completly. And I have some sympathy.
In general, it is a much better practice to explicitly separate the data
outputs.  The structure is sent in channel while the error code is sent
separately out of channel.  If I was going to write a function that creates
p, I often use something like this:

typedef enum fulfillment_type {SUCCESS, FAILURE} fulfillment;

/*
creates a valid printer_dev at the buffer pointed to by printer -- returns
SUCCESS or FAILURE depending on whether a valid pring_dev could actually be
created
*/
fulfillment create_printer(printer_dev* printer);

In this way, I have an explicitly differentiated the error information from
the data itself.

Object Oriented systems sometimes have dummy objects for this sort of
thing. NullPrinter just eats its data.

The simplest thing to do isn't always the best thing to do.

but it often is. If you can arrange your interface so it is simple to
use it reduces the chances of your caller making mistakes.
 
T

Tim Harig

ah,ok. That makes sense. The ?: operator yeilds a value. If TH is
saying that ?: usage where the value is ignored is odd then I'll
agree. If he's saying all uses of ?: should be depracated and replaced
with if-else statments then I disagree. TH seems to be using a coding
a style that bans or heavily discourages use of ?:. I think that's a
bad idea.

I am saying both. I am using the former as an argument for the latter. I
don't deny that ?: can be useful. We discourage its use because we have
found that its use was abused more then it was helpful.

This isn't the tragedy that you might assume. You will note that many
languages do not support the ?: construct. They manage to get along fine
without it. We do as well.
 
T

Tim Harig

Tim Harig said:
They don't really loose reability, they just become a little longer:

/* print x if available or print "<nil>" as a substitute */
if (x == NULL)
printf("%s", x);
else
printf("%s", "nil");

That's an exponential increase in the amount of code. Try the
same technique with:

printf("%s: %s([%s, %s])=%s, new state=%s\n",
state?state:"<no-state>",
op?op:"<no-op>",
v1?v1:"<no-v1>",
v2?v2:"<no-v2>",
res?res:"<no-result>",
newstate?newstate:"<no-state>");

1. We don't worry about programmer convenience. We worry about bug free
code. If that requires more lines, then so be it.

2. I have already said (twice now) that I do not mind its usage in output
statements where none of the assignments persist outside of the
output statement block. Used that way, they can have no side
affects.

3. I have already mentioned that we do not allow NULL pointers to permiate
our code. Either alternate values would already be assigned or
their use would be prohibited. Your example above would therefore
never happen within our codebase.
 
T

Tim Harig

I've heard it termed "in band" as in "in band signalling". But I'm
from a comms background. Some protocols emded control information in
the datastream. It can be tricky to get right but handy when done
right. All the various escape sequences like \n and \a can be thought
of as a similar sort of thing. As is \0 in C strings.

Exactly so. Anybody who has written a shell script can confirm how easy it
is to make mistakes in escaping. Escaping within literal strings cannot be
totally avoided; but, we do make efforts. For instance, we use a printf
function wrapper that automatically adds newlines to the end of the output.
yes it's the same sort of thing and can cause problems. I used to work
with someone who banned in completly. And I have some sympathy.

We still require our coders to understand how to use the standard library
functions and any API's that may do the same; but, we design our own
functions not to use "in band signalling". We also provide function
wrappers for the most common library functions fgetc(), malloc(), etc.
Many of these functions also provide additional benefits; ie, our malloc()
wrapper provides additional debugging code.
Object Oriented systems sometimes have dummy objects for this sort of
thing. NullPrinter just eats its data.

I *did* think of using a dummy printer for this; but, I wasn't sure that it
was right for this scenerio and I knew that I would catch more flac for the
idea from this group. I chose not to use a dummy printer here because it seems
to me a better option simply to disable the printing if no printer is
available. This is appropriate for the application user as well as the
coders.
but it often is. If you can arrange your interface so it is simple to
use it reduces the chances of your caller making mistakes.

I do very much agree with the general idea; but, I have previously provided
explanation for why I don't think the simplest thing is appropriate in this
context.
 
S

Seebs

That's an exponential increase in the amount of code. Try the
same technique with:
printf("%s: %s([%s, %s])=%s, new state=%s\n",
state?state:"<no-state>",
op?op:"<no-op>",
v1?v1:"<no-v1>",
v2?v2:"<no-v2>",
res?res:"<no-result>",
newstate?newstate:"<no-state>");
1. We don't worry about programmer convenience. We worry about bug free
code. If that requires more lines, then so be it.

More lines of code can, often, result in more bugs.
2. I have already said (twice now) that I do not mind its usage in output
statements where none of the assignments persist outside of the
output statement block. Used that way, they can have no side
affects.

Ahh, missed that.
3. I have already mentioned that we do not allow NULL pointers to permiate
our code. Either alternate values would already be assigned or
their use would be prohibited. Your example above would therefore
never happen within our codebase.

So far as I can tell, this remains 100% pure handwaving. Prohibiting their
use just means someone is still checking them, and then you have a bunch
of asserts. This means that, in production, either you have completely
unchecked use of null pointers, or your software instantly crashes with
error messages meaningless to the user without any attempt at saving data.

Neither of these is a good choice. Robust code should be checking *anyway*;
that would be the way to generate genuinely bug-free code.

There is no good way for any non-trivial program to be sure it's covered all
possible code paths during testing; as a result, you can never be sure the
asserts caught everything. Leaving them in means that your program aborts
without warning rather than making any effort at all to recover. Removing
them probably means the same thing, if you're lucky.

-s
 
N

Nick

Keith Thompson said:
I don't think I've ever seen C code in which a macro is #define'd, then
used, then immediately #undef'ed. I can see the argument in favor of
doing it, but in practice most macros are global anyway.

I've done it in code where I'm including the same file to define an enum
and the array to turn it back into text, for example.

#define the macro for one expansion, #include the file, #undef, #define
to new value, #include, #undef (in case I need to do it again later).
 
N

Nick

Seebs said:
It overlooks nothing.

THAT IS THE SPECIFICATION OF THE LANGUAGE.


No, free won't need to read that info, because the first thing it would do
is check to see whether the pointer is a null pointer, and if it is, return
without doing anything.

Ooh no, you mustn't do that. A properly written free will check whether
the pointer is a null pointer and if not do a huge amount of block
manipulation to free the space and consolidate the space. Then it will
return from the two conditions in the same place.

That's much clearer and less confusing.

<gdr>
 

Ask a Question

Want to reply to this thread or ask your own question?

You'll need to choose a username for the site, which only take a couple of moments. After that, you can post your question and our members will help you out.

Ask a Question

Members online

No members online now.

Forum statistics

Threads
473,744
Messages
2,569,484
Members
44,903
Latest member
orderPeak8CBDGummies

Latest Threads

Top