Code review requested

[Ben Bacarisse]

Note - I personally don't think that "defects per KLOC" are
particularly useful metrics.

However, people do study them, and studies of common defects gives
useful information about techniques to reduce the likelihood and
impact of errors.

That's what I wanted to know; specifically if any of these people have
published results of these studies (that you know of)? I could then
look to see if any relate to C's declaration syntax. It gets a bad
press so I imagine that it might well have been included as a target of
one of these studies.

There are endless references on the internet about common errors in C
programming. Many of these can be identified by compilers, especially
with higher warning levels, or with tools like lint. Many others can
be eliminated by using a series of quality development techniques such
as strict code styles, language limitation (for embedded systems it is
common to have an outright ban on malloc and friends - completely
eliminating almost all potential dynamic memory errors), code review,
etc. Banning declarations like "int *a, b, c;" is just part of the
process of producing high-quality code with low defect rates.

If you want to have any kind of statistic about what impact this
particular type of declaration has on average defects per KLOC, then
your guess is as good as mine. It is simply an example of mildly
obfuscated code that makes the source harder to follow - and therefore
more likely to contain bugs.

I think we've reached the end of the road. I accept that you believe it
to be patently true, but I've always hankered after putting these
matters onto a firmer footing.

<snip>

 

[Malcolm McLean]

בת×ריך ×™×•× ×©× ×™,24 בספטמבר 2012 16:43:32 UTC+1, מ×ת David Brown:

On 24/09/2012 16:14, Ben Bacarisse wrote:

Banning declarations like "int *a, b, c;" is just part of the
process of producing high-quality code with low defect rates.

Code that's easy to read is easy to modify and scan for bugs.
However I doubt that many errors are directly caused by mixed
declarations of this type. Confusing a pointer with an integer
is not the sort of problem that's likely to come up very often.
Usually the compiler will complain first.

 

[Ian Collins]

I've never found them essential.

The are in the case of an opaque type.

 

[Keith Thompson]

On Sep 24, 10:05 am, David Brown <[email protected]>
wrote: [...]

Typedefs are even more important if you are talking about function pointers.

I'll agree typedefs are necessary for function pointers

Or you can declare a typedef for the function type:

typedef void func_type(void);
func_type *func_ptr_obj;

 

[James Kuyper]

The are in the case of an opaque type.

Could you expound on that a little more? I've written code using opaque
types that were always referred to in the user code as struct
myOpaqueType*, with no definition provided about what the members of
that struct were. I made no use of a typedef while writing such code -
was I doing something wrong?

 

[Ben Bacarisse]

בת×ריך ×™×•× ×©× ×™, 24 בספטמבר 2012 16:43:32 UTC+1, מ×ת David Brown:

It's better to snip attributions if you don't quote anything written by
that person (in this case me). Not everyone knows ow to count the >s.

<snip>

 

[Keith Thompson]

(Except that a pointer typedef could help avoid the error where you type
int* a,b,c instead of int *a,*b,*c. I'm writing a bit of C at the minute and
this dilemma constantly crops up: does the "*" belong with the type, or with
the identifier being declared?

[...]

A good solution to that is to declare each object on a line by itself:

int *a;
int *b;
int *c;

Then, at least for this relatively simple case, it doesn't matter
whether you write "int *a;" or "int* a;".

I prefer "int *a;" myself, partly because it reflects the language
grammar. What it really means is that "*a" is of type "int"; it
follows from that that "a" is of type "int*". (And yes, that's an
odd way to construct a declaration, and not the way I would have
defined it, but that's the language.)

(Most C++ programmers seem to prefer "int* a;", probably because
Stroustrup uses that form in his books.)

 

[James Kuyper]

The are in the case of an opaque type.

Could you expound on that a little more? I've written code using opaque
types that were always referred to in the user code as struct
myOpaqueType*, with no definition provided about what the members of
that struct were. I made no use of a typedef while writing such code -
was I doing something wrong?

 

[Keith Thompson]

(Except that a pointer typedef could help avoid the error where you type
int* a,b,c instead of int *a,*b,*c. I'm writing a bit of C at the minute and
this dilemma constantly crops up: does the "*" belong with the type, or with
the identifier being declared?

[...]

A good solution to that is to declare each object on a line by itself:

int *a;
int *b;
int *c;

Then, at least for this relatively simple case, it doesn't matter
whether you write "int *a;" or "int* a;".

I prefer "int *a;" myself, partly because it reflects the language
grammar. What it really means is that "*a" is of type "int"; it
follows from that that "a" is of type "int*". (And yes, that's an
odd way to construct a declaration, and not the way I would have
defined it, but that's the language.)

(Most C++ programmers seem to prefer "int* a;", probably because
Stroustrup uses that form in his books.)

 

[Ian Collins]

Could you expound on that a little more? I've written code using opaque
types that were always referred to in the user code as struct
myOpaqueType*, with no definition provided about what the members of
that struct were. I made no use of a typedef while writing such code -
was I doing something wrong?

Something like pthread's pthread_t type. On some platforms it is a
struct, on others an integer type.

In other words types where the underlying type may be a struct, or it
may be something else.

 

[Keith Thompson]

(This is getting a bit off-topic, but I'll ask anyway.)

Why would you have a case against typedefs for pointer types? You
seem to have a general dislike of typedefs!

I find pointer typedefs are often useful - and in some cases they are
essential. I don't meant they are essential to writing code that will
compile - but they are sometimes essential to writing code that is
clear and unambiguous to the reader and the writer.

I've seen numerous articles here arguing against the use of typedefs for
pointer types. (I've also seen Windows code that uses them extensively.)

The semantics of pointer types and non-pointer types are so different
that I think it's important to be very explicit about which one you're
using. And "some_type*" is a sufficiently simple concept (for someone
familiar with C syntax and semantics) that I don't see much point in
hiding it behind a new name. It already has a name: "some_type*".

C's declaration syntax is admittedly obscure at times; the fact that the
type of a pointer to an array of 10 ints, is "int(*)[10]" is annoying.
But if I were using such a type, I'd consider typedef'ing the array
type, not the pointer type:

typedef int arr_10[10];
arr_10 *ptr;

I wouldn't quite say that I have a general dislike of typedefs.
I just think they're overused. If a typedef creates a truly abstract
type like "FILE", I have no problem with it. If it creates a name
for a type, when the underlying type can vary from one implementation
to another, or if the choice of name adds significant information,
that's fine too; examples are size_t and int32_t.

I don't necessarily *like* the way C defines types, but I'd rather
deal with that than add a layer on top of it to make it look *almost*
like what I would have preferred.

In my work, const and volatile qualifiers are common, so it is not
uncommon for the type of a pointer to contain many parts. This makes
it hard to read, and easy to get wrong:

uint8_t volatile * const p = ...;

Is that a constant pointer to volatile data, or a volatile pointer to
constant data, or a pointer to a volatile constant?

typedef volatile uint8_t *pVolUint8_t;
const pVolUint8_t p = ...;

Here there is no doubt what is volatile, and what is const, and how
the parts are related.

That's a good point. Personally, I haven't run into many cases where
that's much of an issue.

[...]

Typedefs are even more important if you are talking about function pointers.

An alternative is to typedef the function type, and then use the
explicit "*" for the pointer-to-function type. I don't expect to
convince you that that's better.

Naming conventions are also very useful here - all my pointer types
being with "p" (or "P" if you prefer), and all function pointer types
begin with "f" (or "F" if you prefer).

That's fine until you mix your code with code written by somebody else
who doesn't follow your conventions -- such as the C standard library.

 

[Nick Keighley]

No, it's not Hungarian - neither the original (fairly sensible)
Hungarian notation, nor the really unpleasant misunderstanding that
people often think of as Hungarian notation.  It's just a helpful
indication that makes some code clearer - and there is no significant
difference from calling your "pointer to a compare function" "fCompare"
or "compareFunc".

to me it seems odd to hide the "pointerness" by using a typedef and
then revealing the pointerness by using an obscure naming convention.

 

[Walter Banks]

? If the condition does
? not actually need to be re-evaluated, most modern compilers can be
? counted on to detect that fact (more reliably than I can), and use an
? unnamed temporary for the same purpose that a named boolean variable
? would have been used for.

I wish I knew any compiler than could do that for the 8-bit targets
(ST7-on-steroids, 8051-siblings, PIC18-and-then-some) that I often code for.

In our 8 bitcompilers we detect common conditional expressions but
don't ever save the results for future use for three reasons

1) It doesn't happen very often in real application code.
2) It slows down the original evaluation of the expression usually by interfering with data flow in a primary
register
3) Indirect paths to terms that are not declared as volatiles and should be.

It is one of those optimizations where the programmer has to be disciplined and results are limited and the
support is high. It comes up from time to time in internal product reviews and then we review the metrics of
actual applications and discover that it would be rarely useful

I doubt it would use bit variables
and instructions often availabl. Hence, *if* speed or size is more relevant
than clarity, using an explicit intermediary bit variable would typically be
beneficial on these targets.

We use bit instructions in our code generation and support single bit booleans on all processors with these
instructions. Our ram allocator packs packs booleans into bytes if we can.

Walter Banks
Byte Craft Limited

 

[David Thompson]

(Below was one huge line, qp'ed. Yuck. Googrope strikes again. Please
see if there is some way to fix this.)

I have been thinking of this, but i picked assert(0) because I thought

assert(!"Some useful diagnostic") is not standard-conforming. However,
if I compile with gcc and the -std=c99 option i get no warnings, and
thinking about it, the expression !"Some useful diagnostic" probably
also converts to a 'scalar expression' as required by the C99
standard, so is probably completely standard-conforming (please
correct me if I'm wrong).
OT1H, that's not strong evidence. Even with -std= (or -ansi), gcc is
not very aggressive about warnings. To get the most help it can give
you, use at least -Wall and -pedantic, and consider -Wextra. Even
then, there are some errors a compiler can't detect and diagnose.

However, it is indeed standard in C99+. Formally it wasn't in C89;
that used a pseudo-prototype void assert(int expression) which was
unclear for pointers and actually wrong for f.p. But in practice
everyone implemented it the traditional and well-known right way.

 

[Philip Lantz]

... it is indeed standard in C99+. Formally it wasn't in C89;
that used a pseudo-prototype void assert(int expression) which was
unclear for pointers and actually wrong for f.p.

!"string" and !fp are int and always have been, so they were fine to use
with assert even in C89.

I agree that assert("string") wasn't allowed by C89, but that's pretty
useless anyway. assert(fp) was problematic, because it was allowed but
likely wouldn't do what the programmer wanted.

 

[Bart Vandewoestyne]

[...]
My only big comment... A lot of the code is boilerplate to deal with
a large number of structure types (well, unions within a structure). I
would probably search for some way to simplify this. I don't know the
book, so it's possible you have to do it this way.

I know this thread is from a while ago, but i remember quite a heavy discussion about the structs... Today, I came across an article in Dr. Dobb's with the title 'Discriminated Unions': http://www.drdobbs.com/cpp/discriminated-unions/240009296?pgno=2

It seems to me that the author of the code from Appel's 'Modern Compiler Construction in C' heavily uses this technique... Personally, as quite a C-novice, I didn't know it... but since Dan Saks wrote a blogpost on it, I canimagine that discriminated unions is quite a common thing used in C programming. Searching for 'discriminated' in the latest C standard doesn't giveany results. Does anybody know what book introduced the term 'discriminated unions'? I would like to read more about it...

Regards,
Bart

 

[Ben Bacarisse]

[...]
My only big comment... A lot of the code is boilerplate to deal with
a large number of structure types (well, unions within a structure). I
would probably search for some way to simplify this. I don't know the
book, so it's possible you have to do it this way.

I know this thread is from a while ago, but i remember quite a heavy
discussion about the structs... Today, I came across an article in
Dr. Dobb's with the title 'Discriminated Unions':
http://www.drdobbs.com/cpp/discriminated-unions/240009296?pgno=2

It seems to me that the author of the code from Appel's 'Modern
Compiler Construction in C' heavily uses this technique...
Personally, as quite a C-novice, I didn't know it... but since Dan
Saks wrote a blogpost on it, I can imagine that discriminated unions
is quite a common thing used in C programming. Searching for
'discriminated' in the latest C standard doesn't give any results.

It goes by other names (tagged unions, for example) but, either way, you
won't find a reference to it in the C standard because it's not a C
language feature. C is a relatively low-level language, so to get the
same effect you have to do the work yourself and make sure that your
code does not "break the rules".

Does anybody know what book introduced the term 'discriminated
unions'? I would like to read more about it...

I can't help here, but I can point out that the term is not universal.
For example, Pascal had them, but they are called variant records.
Algol 68 had them and they are just called unions! (I say, had simply
because these languages are old -- they still have all the features they
ever had of course.)

 

[Bart Vandewoestyne]

It goes by other names (tagged unions, for example) but, either way, you
won't find a reference to it in the C standard because it's not a C
language feature. C is a relatively low-level language, so to get the
same effect you have to do the work yourself and make sure that your
code does not "break the rules".

Indeed. It seems like the concept of 'tagged unions' is more like a computer science abstract data type kind of thing. See e.g. the article on wikipedia:

http://en.wikipedia.org/wiki/Tagged_union

Different languages seem to have different ways of implementing tagged unions, and the way it's done in C is just one of them.

I can't help here, but I can point out that the term is not universal.
For example, Pascal had them, but they are called variant records.
Algol 68 had them and they are just called unions! (I say, had simply
because these languages are old -- they still have all the features they
ever had of course.)

Doing a search on books.google.com leads me to page 44 of the book 'C programming FAQs: frequently asked questions' where tagged unions are shortly mentioned in an answer to a question. Other than that, I can't really find much C-specific books that mention tagged unions. However, there do appear a lot of Data Structures books in the search results...

Interesting. I learned something today!

Regards,
Bart

 

[Keith Thompson]

Doing a search on books.google.com leads me to page 44 of the book 'C
programming FAQs: frequently asked questions' where tagged unions are
shortly mentioned in an answer to a question. Other than that, I
can't really find much C-specific books that mention tagged unions.
However, there do appear a lot of Data Structures books in the search
results...

An abbreviated version of that book is available at
<http://www.c-faq.com/>; it looks like you're referring to question
2.21. It's an extraordinarily useful web site.

 

[Tim Rentsch]

Could you expound on that a little more? I've written code using opaque
types that were always referred to in the user code as struct
myOpaqueType*, with no definition provided about what the members of
that struct were. I made no use of a typedef while writing such code -
was I doing something wrong?

If an interface declares, say, a parameter to be of type
'struct myOpaqueType*', then the type of the parameter is
not opaque - it's immediately evident that the parameter is
a pointer, and what the pointer is pointing to is a struct.
For another example, suppose there is a header file that
has

struct example_1 { ... };
extern struct example_1 some_examples[];

The type of some_examples (ie, of the actual object) is not
known, because the array bound isn't known, but certainly
we wouldn't say the type of some_examples is opaque. To be
opaque, the type used on the declaration in question should
convey /no information/ about the type of whatever is being
declared.